[libvirt] [PATCH v3 00/15] Misc build refactoring / isolation work
This was triggered by the recent Fedora change to add '-z defs' to RPM builds by default which breaks libvirt. Various make rule changes can fix much of the problem, but it also requires source refactoring to get rid of places where virt drivers directly call into the storage/network drivers. Co-incidentally this work will also be useful in allowing us to separate out drivers to distinct daemons. In v3: - Fixed a few build problems identified by travis In v2: - Fixed header file name comment - Resolve conflicts - Fix unit tests - Fix bisectable build by moving libvirt_lxc build patch earlier - Update syntax check header include rule Daniel P. Berrangé (15): storage: extract storage file backend from main storage driver backend storage: move storage file backend framework into util directory rpc: don't link in second copy of RPC code to libvirtd & lockd plugin build: link libvirt_lxc against libvirt.so conf: introduce callback registration for domain net device allocation conf: expand network device callbacks to cover bandwidth updates qemu: replace networkGetNetworkAddress with public API calls conf: expand network device callbacks to cover resolving NIC type network: remove conditional declarations conf: move virStorageTranslateDiskSourcePool into domain conf storage: export virStoragePoolLookupByTargetPath as a public API build: explicitly link all modules with libvirt.so build: provide a AM_FLAGS_MOD for loadable modules build: passing the "-z defs" linker flag to prevent undefined symbols cfg: forbid includes of headers in network and storage drivers again cfg.mk | 2 +- configure.ac | 1 + daemon/Makefile.am | 3 +- include/libvirt/libvirt-storage.h | 2 + m4/virt-linker-no-undefined.m4 | 32 ++ po/POTFILES.in | 2 +- src/Makefile.am | 150 ++++---- src/bhyve/bhyve_command.c | 7 +- src/conf/domain_conf.c | 355 +++++++++++++++++++ src/conf/domain_conf.h | 71 ++++ src/driver-storage.h | 5 + src/libvirt-storage.c | 40 +++ src/libvirt_private.syms | 29 ++ src/libvirt_public.syms | 6 + src/libvirt_remote.syms | 11 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 7 +- src/lxc/lxc_driver.c | 5 +- src/lxc/lxc_process.c | 7 +- src/network/bridge_driver.c | 124 +------ src/network/bridge_driver.h | 72 ---- src/qemu/qemu_alias.c | 3 +- src/qemu/qemu_command.c | 1 - src/qemu/qemu_domain.c | 3 - src/qemu/qemu_domain_address.c | 3 +- src/qemu/qemu_driver.c | 15 +- src/qemu/qemu_hotplug.c | 18 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_process.c | 115 +++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 17 +- src/remote_protocol-structs | 7 + src/security/virt-aa-helper.c | 2 - src/storage/storage_backend.c | 66 ---- src/storage/storage_backend.h | 75 ---- src/storage/storage_backend_fs.c | 8 +- src/storage/storage_backend_gluster.c | 4 +- src/storage/storage_driver.c | 256 +------------- src/storage/storage_driver.h | 3 - src/storage/storage_source.c | 645 ---------------------------------- src/storage/storage_source.h | 59 ---- src/util/virstoragefile.c | 609 +++++++++++++++++++++++++++++++- src/util/virstoragefile.h | 32 ++ src/util/virstoragefilebackend.c | 108 ++++++ src/util/virstoragefilebackend.h | 104 ++++++ src/vz/vz_sdk.c | 1 - tests/Makefile.am | 11 +- tests/qemuxml2argvtest.c | 4 + tests/virstoragetest.c | 1 - tools/Makefile.am | 1 + 50 files changed, 1679 insertions(+), 1432 deletions(-) create mode 100644 m4/virt-linker-no-undefined.m4 delete mode 100644 src/storage/storage_source.c delete mode 100644 src/storage/storage_source.h create mode 100644 src/util/virstoragefilebackend.c create mode 100644 src/util/virstoragefilebackend.h -- 2.14.3
The storage driver backends are serving the public storage pools API, while the storage file backends are serving the internal QEMU driver and / or libvirt utility code. To prep for moving this storage file backend framework into the utility code, split out the backend definitions. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/storage/storage_backend.c | 66 --------------------- src/storage/storage_backend.h | 75 ----------------------- src/storage/storage_backend_fs.c | 7 ++- src/storage/storage_backend_gluster.c | 3 +- src/storage/storage_source.c | 2 +- src/storage/storage_source_backend.c | 108 ++++++++++++++++++++++++++++++++++ src/storage/storage_source_backend.h | 104 ++++++++++++++++++++++++++++++++ 9 files changed, 221 insertions(+), 146 deletions(-) create mode 100644 src/storage/storage_source_backend.c create mode 100644 src/storage/storage_source_backend.h diff --git a/po/POTFILES.in b/po/POTFILES.in index cbf2accba4..ea99f61677 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -187,6 +187,7 @@ src/storage/storage_backend_vstorage.c src/storage/storage_backend_zfs.c src/storage/storage_driver.c src/storage/storage_source.c +src/storage/storage_source_backend.c src/storage/storage_util.c src/test/test_driver.c src/uml/uml_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index 79adc9ba51..99f9bfb6f2 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1065,6 +1065,7 @@ STORAGE_DRIVER_BACKEND_SOURCES = \ STORAGE_DRIVER_SOURCES = \ storage/storage_driver.h storage/storage_driver.c \ storage/storage_source.h storage/storage_source.c \ + storage/storage_source_backend.h storage/storage_source_backend.c \ $(STORAGE_DRIVER_BACKEND_SOURCES) \ storage/storage_util.h storage/storage_util.c diff --git a/src/storage/storage_backend.c b/src/storage/storage_backend.c index 5a8c4f7f6a..053f4ecf26 100644 --- a/src/storage/storage_backend.c +++ b/src/storage/storage_backend.c @@ -78,8 +78,6 @@ VIR_LOG_INIT("storage.storage_backend"); static virStorageBackendPtr virStorageBackends[VIR_STORAGE_BACKENDS_MAX]; static size_t virStorageBackendsCount; -static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; -static size_t virStorageFileBackendsCount; #define STORAGE_BACKEND_MODULE_DIR LIBDIR "/libvirt/storage-backend" @@ -179,27 +177,6 @@ virStorageBackendRegister(virStorageBackendPtr backend) } -int -virStorageBackendFileRegister(virStorageFileBackendPtr backend) -{ - VIR_DEBUG("Registering storage file backend '%s' protocol '%s'", - virStorageTypeToString(backend->type), - virStorageNetProtocolTypeToString(backend->protocol)); - - if (virStorageFileBackendsCount >= VIR_STORAGE_BACKENDS_MAX) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("Too many drivers, cannot register storage file " - "backend '%s'"), - virStorageTypeToString(backend->type)); - return -1; - } - - virStorageFileBackends[virStorageFileBackendsCount] = backend; - virStorageFileBackendsCount++; - return 0; -} - - virStorageBackendPtr virStorageBackendForType(int type) { @@ -213,46 +190,3 @@ virStorageBackendForType(int type) type, NULLSTR(virStoragePoolTypeToString(type))); return NULL; } - - -virStorageFileBackendPtr -virStorageFileBackendForTypeInternal(int type, - int protocol, - bool report) -{ - size_t i; - - 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]; - } - } - - if (!report) - return NULL; - - if (type == VIR_STORAGE_TYPE_NETWORK) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing storage backend for network files " - "using %s protocol"), - virStorageNetProtocolTypeToString(protocol)); - } else { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("missing storage backend for '%s' storage"), - virStorageTypeToString(type)); - } - - return NULL; -} - - -virStorageFileBackendPtr -virStorageFileBackendForType(int type, - int protocol) -{ - return virStorageFileBackendForTypeInternal(type, protocol, true); -} diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h index 8dbe344149..5587f84407 100644 --- a/src/storage/storage_backend.h +++ b/src/storage/storage_backend.h @@ -110,83 +110,8 @@ struct _virStorageBackend { virStorageBackendPtr virStorageBackendForType(int type); -/* ------- virStorageFile backends ------------ */ -typedef struct _virStorageFileBackend virStorageFileBackend; -typedef virStorageFileBackend *virStorageFileBackendPtr; - -struct _virStorageDriverData { - virStorageFileBackendPtr backend; - void *priv; - - uid_t uid; - gid_t gid; -}; - -typedef int -(*virStorageFileBackendInit)(virStorageSourcePtr src); - -typedef void -(*virStorageFileBackendDeinit)(virStorageSourcePtr src); - -typedef int -(*virStorageFileBackendCreate)(virStorageSourcePtr src); - -typedef int -(*virStorageFileBackendUnlink)(virStorageSourcePtr src); - -typedef int -(*virStorageFileBackendStat)(virStorageSourcePtr src, - struct stat *st); - -typedef ssize_t -(*virStorageFileBackendRead)(virStorageSourcePtr src, - size_t offset, - size_t len, - char **buf); - -typedef const char * -(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src); - -typedef int -(*virStorageFileBackendAccess)(virStorageSourcePtr src, - int mode); - -typedef int -(*virStorageFileBackendChown)(const virStorageSource *src, - uid_t uid, - gid_t gid); - -virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol); -virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type, - int protocol, - bool report); - - -struct _virStorageFileBackend { - int type; - int protocol; - - /* All storage file callbacks may be omitted if not implemented */ - - /* The following group of callbacks is expected to set a libvirt - * error on failure. */ - virStorageFileBackendInit backendInit; - virStorageFileBackendDeinit backendDeinit; - virStorageFileBackendRead storageFileRead; - virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier; - - /* The following group of callbacks is expected to set errno - * and return -1 on error. No libvirt error shall be reported */ - virStorageFileBackendCreate storageFileCreate; - virStorageFileBackendUnlink storageFileUnlink; - virStorageFileBackendStat storageFileStat; - virStorageFileBackendAccess storageFileAccess; - virStorageFileBackendChown storageFileChown; -}; - int virStorageBackendDriversRegister(bool allmodules); int virStorageBackendRegister(virStorageBackendPtr backend); -int virStorageBackendFileRegister(virStorageFileBackendPtr backend); #endif /* __VIR_STORAGE_BACKEND_H__ */ diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index b3bae38437..c528c1dae5 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -37,6 +37,7 @@ #include "virerror.h" #include "storage_backend_fs.h" +#include "storage_source_backend.h" #include "storage_util.h" #include "storage_conf.h" #include "virstoragefile.h" @@ -911,13 +912,13 @@ virStorageBackendFsRegister(void) return -1; #endif /* WITH_STORAGE_FS */ - if (virStorageBackendFileRegister(&virStorageFileBackendFile) < 0) + if (virStorageFileBackendRegister(&virStorageFileBackendFile) < 0) return -1; - if (virStorageBackendFileRegister(&virStorageFileBackendBlock) < 0) + if (virStorageFileBackendRegister(&virStorageFileBackendBlock) < 0) return -1; - if (virStorageBackendFileRegister(&virStorageFileBackendDir) < 0) + if (virStorageFileBackendRegister(&virStorageFileBackendDir) < 0) return -1; return 0; diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 6e4f19f76d..68d9c726e9 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -24,6 +24,7 @@ #include <glusterfs/api/glfs.h> #include "storage_backend_gluster.h" +#include "storage_source_backend.h" #include "storage_conf.h" #include "viralloc.h" #include "virerror.h" @@ -866,7 +867,7 @@ virStorageBackendGlusterRegister(void) if (virStorageBackendRegister(&virStorageBackendGluster) < 0) return -1; - if (virStorageBackendFileRegister(&virStorageFileBackendGluster) < 0) + if (virStorageFileBackendRegister(&virStorageFileBackendGluster) < 0) return -1; return 0; diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c index 170ab755b7..a5eefe5032 100644 --- a/src/storage/storage_source.c +++ b/src/storage/storage_source.c @@ -26,7 +26,7 @@ #include "virerror.h" #include "storage_source.h" -#include "storage_backend.h" +#include "storage_source_backend.h" #include "viralloc.h" #include "virlog.h" #include "virstring.h" diff --git a/src/storage/storage_source_backend.c b/src/storage/storage_source_backend.c new file mode 100644 index 0000000000..e093c04989 --- /dev/null +++ b/src/storage/storage_source_backend.c @@ -0,0 +1,108 @@ +/* + * storage_source_backend.c: internal storage source backend contract + * + * Copyright (C) 2007-2018 Red Hat, Inc. + * Copyright (C) 2007-2008 Daniel P. Berrange + * + * 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/>. + * + * Author: Daniel P. Berrange <berrange@redhat.com> + */ + +#include <config.h> + +#include <string.h> +#include <sys/stat.h> + +#include "datatypes.h" +#include "virerror.h" +#include "viralloc.h" +#include "internal.h" +#include "storage_source_backend.h" +#include "virlog.h" +#include "virfile.h" +#include "configmake.h" + +#define VIR_FROM_THIS VIR_FROM_STORAGE + +VIR_LOG_INIT("storage.storage_source_backend"); + +#define VIR_STORAGE_BACKENDS_MAX 20 + +static virStorageFileBackendPtr virStorageFileBackends[VIR_STORAGE_BACKENDS_MAX]; +static size_t virStorageFileBackendsCount; + + +int +virStorageFileBackendRegister(virStorageFileBackendPtr backend) +{ + VIR_DEBUG("Registering storage file backend '%s' protocol '%s'", + virStorageTypeToString(backend->type), + virStorageNetProtocolTypeToString(backend->protocol)); + + if (virStorageFileBackendsCount >= VIR_STORAGE_BACKENDS_MAX) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Too many drivers, cannot register storage file " + "backend '%s'"), + virStorageTypeToString(backend->type)); + return -1; + } + + virStorageFileBackends[virStorageFileBackendsCount] = backend; + virStorageFileBackendsCount++; + return 0; +} + +virStorageFileBackendPtr +virStorageFileBackendForTypeInternal(int type, + int protocol, + bool report) +{ + size_t i; + + 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]; + } + } + + if (!report) + return NULL; + + if (type == VIR_STORAGE_TYPE_NETWORK) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing storage backend for network files " + "using %s protocol"), + virStorageNetProtocolTypeToString(protocol)); + } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("missing storage backend for '%s' storage"), + virStorageTypeToString(type)); + } + + return NULL; +} + + +virStorageFileBackendPtr +virStorageFileBackendForType(int type, + int protocol) +{ + return virStorageFileBackendForTypeInternal(type, protocol, true); +} diff --git a/src/storage/storage_source_backend.h b/src/storage/storage_source_backend.h new file mode 100644 index 0000000000..3af2a5f380 --- /dev/null +++ b/src/storage/storage_source_backend.h @@ -0,0 +1,104 @@ +/* + * storage_source_backend.h: internal storage source backend contract + * + * Copyright (C) 2007-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_SOURCE_BACKEND_H__ +# define __VIR_STORAGE_SOURCE_BACKEND_H__ + +# include <sys/stat.h> + +# include "virstoragefile.h" + +/* ------- virStorageFile backends ------------ */ +typedef struct _virStorageFileBackend virStorageFileBackend; +typedef virStorageFileBackend *virStorageFileBackendPtr; + +struct _virStorageDriverData { + virStorageFileBackendPtr backend; + void *priv; + + uid_t uid; + gid_t gid; +}; + +typedef int +(*virStorageFileBackendInit)(virStorageSourcePtr src); + +typedef void +(*virStorageFileBackendDeinit)(virStorageSourcePtr src); + +typedef int +(*virStorageFileBackendCreate)(virStorageSourcePtr src); + +typedef int +(*virStorageFileBackendUnlink)(virStorageSourcePtr src); + +typedef int +(*virStorageFileBackendStat)(virStorageSourcePtr src, + struct stat *st); + +typedef ssize_t +(*virStorageFileBackendRead)(virStorageSourcePtr src, + size_t offset, + size_t len, + char **buf); + +typedef const char * +(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src); + +typedef int +(*virStorageFileBackendAccess)(virStorageSourcePtr src, + int mode); + +typedef int +(*virStorageFileBackendChown)(const virStorageSource *src, + uid_t uid, + gid_t gid); + +virStorageFileBackendPtr virStorageFileBackendForType(int type, int protocol); +virStorageFileBackendPtr virStorageFileBackendForTypeInternal(int type, + int protocol, + bool report); + + +struct _virStorageFileBackend { + int type; + int protocol; + + /* All storage file callbacks may be omitted if not implemented */ + + /* The following group of callbacks is expected to set a libvirt + * error on failure. */ + virStorageFileBackendInit backendInit; + virStorageFileBackendDeinit backendDeinit; + virStorageFileBackendRead storageFileRead; + virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier; + + /* The following group of callbacks is expected to set errno + * and return -1 on error. No libvirt error shall be reported */ + virStorageFileBackendCreate storageFileCreate; + virStorageFileBackendUnlink storageFileUnlink; + virStorageFileBackendStat storageFileStat; + virStorageFileBackendAccess storageFileAccess; + virStorageFileBackendChown storageFileChown; +}; + +int virStorageFileBackendRegister(virStorageFileBackendPtr backend); + +#endif /* __VIR_STORAGE_SOURCE_BACKEND_H__ */ -- 2.14.3
The QEMU driver loadable module needs to be able to resolve all ELF symbols it references against libvirt.so. Some of its symbols can only be resolved against the storage_driver.so loadable module which creates a hard dependancy between them. By moving the storage file backend framework into the util directory, this gets included directly in the libvirt.so library. The actual backend implementations are still done as loadable modules, so this doesn't re-add deps on gluster libraries. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- po/POTFILES.in | 3 +- src/Makefile.am | 3 +- src/libvirt_private.syms | 19 + src/qemu/qemu_domain.c | 1 - src/qemu/qemu_driver.c | 1 - src/security/virt-aa-helper.c | 2 - src/storage/storage_backend_fs.c | 3 +- src/storage/storage_backend_gluster.c | 3 +- src/storage/storage_source.c | 645 --------------------- src/storage/storage_source.h | 59 -- src/util/virstoragefile.c | 609 ++++++++++++++++++- src/util/virstoragefile.h | 32 + .../virstoragefilebackend.c} | 4 +- .../virstoragefilebackend.h} | 8 +- tests/virstoragetest.c | 1 - 15 files changed, 669 insertions(+), 724 deletions(-) delete mode 100644 src/storage/storage_source.c delete mode 100644 src/storage/storage_source.h rename src/{storage/storage_source_backend.c => util/virstoragefilebackend.c} (96%) rename src/{storage/storage_source_backend.h => util/virstoragefilebackend.h} (94%) diff --git a/po/POTFILES.in b/po/POTFILES.in index ea99f61677..2859554690 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -186,8 +186,6 @@ src/storage/storage_backend_sheepdog.c src/storage/storage_backend_vstorage.c src/storage/storage_backend_zfs.c src/storage/storage_driver.c -src/storage/storage_source.c -src/storage/storage_source_backend.c src/storage/storage_util.c src/test/test_driver.c src/uml/uml_conf.c @@ -263,6 +261,7 @@ src/util/virsexpr.c src/util/virsocketaddr.c src/util/virstorageencryption.c src/util/virstoragefile.c +src/util/virstoragefilebackend.c src/util/virstring.c src/util/virsysinfo.c src/util/virthreadjob.c diff --git a/src/Makefile.am b/src/Makefile.am index 99f9bfb6f2..54ef81d1ab 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -178,6 +178,7 @@ UTIL_SOURCES = \ util/virsocketaddr.h util/virsocketaddr.c \ util/virstorageencryption.c util/virstorageencryption.h \ util/virstoragefile.c util/virstoragefile.h \ + util/virstoragefilebackend.c util/virstoragefilebackend.h \ util/virstring.h util/virstring.c \ util/virsysinfo.c util/virsysinfo.h util/virsysinfopriv.h \ util/virsystemd.c util/virsystemd.h util/virsystemdpriv.h \ @@ -1064,8 +1065,6 @@ STORAGE_DRIVER_BACKEND_SOURCES = \ STORAGE_DRIVER_SOURCES = \ storage/storage_driver.h storage/storage_driver.c \ - storage/storage_source.h storage/storage_source.c \ - storage/storage_source_backend.h storage/storage_source_backend.c \ $(STORAGE_DRIVER_BACKEND_SOURCES) \ storage/storage_util.h storage/storage_util.c diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 0bce0bbfb2..3ec510bd32 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -2720,24 +2720,39 @@ virStorageAuthDefCopy; virStorageAuthDefFormat; virStorageAuthDefFree; virStorageAuthDefParse; +virStorageFileAccess; virStorageFileCanonicalizePath; virStorageFileChainGetBroken; virStorageFileChainLookup; +virStorageFileChown; +virStorageFileCreate; +virStorageFileDeinit; virStorageFileFeatureTypeFromString; virStorageFileFeatureTypeToString; virStorageFileFormatTypeFromString; virStorageFileFormatTypeToString; +virStorageFileGetBackingStoreStr; virStorageFileGetLVMKey; +virStorageFileGetMetadata; virStorageFileGetMetadataFromBuf; virStorageFileGetMetadataFromFD; virStorageFileGetMetadataInternal; virStorageFileGetRelativeBackingPath; virStorageFileGetSCSIKey; +virStorageFileGetUniqueIdentifier; +virStorageFileInit; +virStorageFileInitAs; virStorageFileIsClusterFS; virStorageFileParseBackingStoreStr; virStorageFileParseChainIndex; virStorageFileProbeFormat; +virStorageFileRead; +virStorageFileReportBrokenChain; virStorageFileResize; +virStorageFileStat; +virStorageFileSupportsAccess; +virStorageFileSupportsSecurityDriver; +virStorageFileUnlink; virStorageIsFile; virStorageIsRelative; virStorageNetHostDefClear; @@ -2776,6 +2791,10 @@ virStorageTypeFromString; virStorageTypeToString; +# util/virstoragefilebackend.h +virStorageFileBackendRegister; + + # util/virstring.h virArgvToString; virAsprintfInternal; diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index c8123ce59b..f6d2723a8a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -56,7 +56,6 @@ #include "locking/domain_lock.h" #include "storage/storage_driver.h" -#include "storage/storage_source.h" #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index d64686df4c..2b8b0682d5 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -100,7 +100,6 @@ #include "viraccessapicheck.h" #include "viraccessapicheckqemu.h" #include "storage/storage_driver.h" -#include "storage/storage_source.h" #include "virhostdev.h" #include "domain_capabilities.h" #include "vircgroup.h" diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c index f7ccae0b02..d5f3e858fa 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -57,8 +57,6 @@ #include "virgettext.h" #include "virhostdev.h" -#include "storage/storage_source.h" - #define VIR_FROM_THIS VIR_FROM_SECURITY static char *progname; diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c index c528c1dae5..9b0fcf92c5 100644 --- a/src/storage/storage_backend_fs.c +++ b/src/storage/storage_backend_fs.c @@ -37,10 +37,9 @@ #include "virerror.h" #include "storage_backend_fs.h" -#include "storage_source_backend.h" #include "storage_util.h" #include "storage_conf.h" -#include "virstoragefile.h" +#include "virstoragefilebackend.h" #include "vircommand.h" #include "viralloc.h" #include "virxml.h" diff --git a/src/storage/storage_backend_gluster.c b/src/storage/storage_backend_gluster.c index 68d9c726e9..c6cc531e2f 100644 --- a/src/storage/storage_backend_gluster.c +++ b/src/storage/storage_backend_gluster.c @@ -24,12 +24,11 @@ #include <glusterfs/api/glfs.h> #include "storage_backend_gluster.h" -#include "storage_source_backend.h" #include "storage_conf.h" #include "viralloc.h" #include "virerror.h" #include "virlog.h" -#include "virstoragefile.h" +#include "virstoragefilebackend.h" #include "virstring.h" #include "viruri.h" #include "storage_util.h" diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c deleted file mode 100644 index a5eefe5032..0000000000 --- a/src/storage/storage_source.c +++ /dev/null @@ -1,645 +0,0 @@ -/* - * storage_source.c: Storage source object accessors to real storage - * - * 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 <sys/types.h> -#include <sys/stat.h> - -#include <errno.h> -#include <string.h> - -#include "virerror.h" -#include "storage_source.h" -#include "storage_source_backend.h" -#include "viralloc.h" -#include "virlog.h" -#include "virstring.h" -#include "virhash.h" - -#define VIR_FROM_THIS VIR_FROM_STORAGE - -VIR_LOG_INIT("storage.storage_source"); - - -static bool -virStorageFileIsInitialized(const virStorageSource *src) -{ - return src && src->drv; -} - - -static virStorageFileBackendPtr -virStorageFileGetBackendForSupportCheck(const virStorageSource *src) -{ - int actualType; - - if (!src) - return NULL; - - if (src->drv) - return src->drv->backend; - - actualType = virStorageSourceGetActualType(src); - - return virStorageFileBackendForTypeInternal(actualType, src->protocol, false); -} - - -static bool -virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) -{ - virStorageFileBackendPtr backend; - - if (!(backend = virStorageFileGetBackendForSupportCheck(src))) - return false; - - return backend->storageFileGetUniqueIdentifier && - backend->storageFileRead && - backend->storageFileAccess; -} - - -/** - * virStorageFileSupportsSecurityDriver: - * - * @src: a storage file structure - * - * Check if a storage file supports operations needed by the security - * driver to perform labelling - */ -bool -virStorageFileSupportsSecurityDriver(const virStorageSource *src) -{ - virStorageFileBackendPtr backend; - - if (!(backend = virStorageFileGetBackendForSupportCheck(src))) - return false; - - return !!backend->storageFileChown; -} - - -/** - * virStorageFileSupportsAccess: - * - * @src: a storage file structure - * - * Check if a storage file supports checking if the storage source is accessible - * for the given vm. - */ -bool -virStorageFileSupportsAccess(const virStorageSource *src) -{ - virStorageFileBackendPtr backend; - - if (!(backend = virStorageFileGetBackendForSupportCheck(src))) - return false; - - return !!backend->storageFileAccess; -} - - -void -virStorageFileDeinit(virStorageSourcePtr src) -{ - if (!virStorageFileIsInitialized(src)) - return; - - if (src->drv->backend && - src->drv->backend->backendDeinit) - src->drv->backend->backendDeinit(src); - - VIR_FREE(src->drv); -} - - -/** - * virStorageFileInitAs: - * - * @src: storage source definition - * @uid: uid used to access the file, or -1 for current uid - * @gid: gid used to access the file, or -1 for current gid - * - * Initialize a storage source to be used with storage driver. Use the provided - * uid and gid if possible for the operations. - * - * Returns 0 if the storage file was successfully initialized, -1 if the - * initialization failed. Libvirt error is reported. - */ -int -virStorageFileInitAs(virStorageSourcePtr src, - uid_t uid, gid_t gid) -{ - int actualType = virStorageSourceGetActualType(src); - if (VIR_ALLOC(src->drv) < 0) - return -1; - - if (uid == (uid_t) -1) - src->drv->uid = geteuid(); - else - src->drv->uid = uid; - - if (gid == (gid_t) -1) - src->drv->gid = getegid(); - else - src->drv->gid = gid; - - if (!(src->drv->backend = virStorageFileBackendForType(actualType, - src->protocol))) - goto error; - - if (src->drv->backend->backendInit && - src->drv->backend->backendInit(src) < 0) - goto error; - - return 0; - - error: - VIR_FREE(src->drv); - return -1; -} - - -/** - * virStorageFileInit: - * - * See virStorageFileInitAs. The file is initialized to be accessed by the - * current user. - */ -int -virStorageFileInit(virStorageSourcePtr src) -{ - return virStorageFileInitAs(src, -1, -1); -} - - -/** - * virStorageFileCreate: Creates an empty storage file via storage driver - * - * @src: file structure pointing to the file - * - * Returns 0 on success, -2 if the function isn't supported by the backend, - * -1 on other failure. Errno is set in case of failure. - */ -int -virStorageFileCreate(virStorageSourcePtr src) -{ - int ret; - - if (!virStorageFileIsInitialized(src) || - !src->drv->backend->storageFileCreate) { - errno = ENOSYS; - return -2; - } - - ret = src->drv->backend->storageFileCreate(src); - - VIR_DEBUG("created storage file %p: ret=%d, errno=%d", - src, ret, errno); - - return ret; -} - - -/** - * virStorageFileUnlink: Unlink storage file via storage driver - * - * @src: file structure pointing to the file - * - * Unlinks the file described by the @file structure. - * - * Returns 0 on success, -2 if the function isn't supported by the backend, - * -1 on other failure. Errno is set in case of failure. - */ -int -virStorageFileUnlink(virStorageSourcePtr src) -{ - int ret; - - if (!virStorageFileIsInitialized(src) || - !src->drv->backend->storageFileUnlink) { - errno = ENOSYS; - return -2; - } - - ret = src->drv->backend->storageFileUnlink(src); - - VIR_DEBUG("unlinked storage file %p: ret=%d, errno=%d", - src, ret, errno); - - return ret; -} - - -/** - * virStorageFileStat: returns stat struct of a file via storage driver - * - * @src: file structure pointing to the file - * @stat: stat structure to return data - * - * Returns 0 on success, -2 if the function isn't supported by the backend, - * -1 on other failure. Errno is set in case of failure. -*/ -int -virStorageFileStat(virStorageSourcePtr src, - struct stat *st) -{ - int ret; - - if (!virStorageFileIsInitialized(src) || - !src->drv->backend->storageFileStat) { - errno = ENOSYS; - return -2; - } - - ret = src->drv->backend->storageFileStat(src, st); - - VIR_DEBUG("stat of storage file %p: ret=%d, errno=%d", - src, ret, errno); - - return ret; -} - - -/** - * virStorageFileRead: read bytes from a file into a buffer - * - * @src: file structure pointing to the file - * @offset: number of bytes to skip in the storage file - * @len: maximum number of bytes read from the storage file - * @buf: buffer to read the data into. (buffer shall be freed by caller) - * - * Returns the count of bytes read on success and -1 on failure, -2 if the - * function isn't supported by the backend. - * Libvirt error is reported on failure. - */ -ssize_t -virStorageFileRead(virStorageSourcePtr src, - size_t offset, - size_t len, - char **buf) -{ - ssize_t ret; - - if (!virStorageFileIsInitialized(src)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("storage file backend not initialized")); - 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)); - return -2; - } - - ret = src->drv->backend->storageFileRead(src, offset, len, buf); - - VIR_DEBUG("read '%zd' bytes from storage '%p' starting at offset '%zu'", - ret, src, offset); - - return ret; -} - - -/* - * virStorageFileGetUniqueIdentifier: Get a unique string describing the volume - * - * @src: file structure pointing to the file - * - * Returns a string uniquely describing a single volume (canonical path). - * The string shall not be freed and is valid until the storage file is - * deinitialized. Returns NULL on error and sets a libvirt error code */ -const char * -virStorageFileGetUniqueIdentifier(virStorageSourcePtr src) -{ - if (!virStorageFileIsInitialized(src)) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("storage file backend not initialized")); - return NULL; - } - - if (!src->drv->backend->storageFileGetUniqueIdentifier) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unique storage file identifier not implemented for " - "storage type %s (protocol: %s)'"), - virStorageTypeToString(src->type), - virStorageNetProtocolTypeToString(src->protocol)); - return NULL; - } - - return src->drv->backend->storageFileGetUniqueIdentifier(src); -} - - -/** - * virStorageFileAccess: Check accessibility of a storage file - * - * @src: storage file to check access permissions - * @mode: accessibility check options (see man 2 access) - * - * Returns 0 on success, -1 on error and sets errno. No libvirt - * error is reported. Returns -2 if the operation isn't supported - * by libvirt storage backend. - */ -int -virStorageFileAccess(virStorageSourcePtr src, - int mode) -{ - if (!virStorageFileIsInitialized(src) || - !src->drv->backend->storageFileAccess) { - errno = ENOSYS; - return -2; - } - - return src->drv->backend->storageFileAccess(src, mode); -} - - -/** - * virStorageFileChown: Change owner of a storage file - * - * @src: storage file to change owner of - * @uid: new owner id - * @gid: new group id - * - * Returns 0 on success, -1 on error and sets errno. No libvirt - * error is reported. Returns -2 if the operation isn't supported - * by libvirt storage backend. - */ -int -virStorageFileChown(const virStorageSource *src, - uid_t uid, - gid_t gid) -{ - if (!virStorageFileIsInitialized(src) || - !src->drv->backend->storageFileChown) { - errno = ENOSYS; - return -2; - } - - VIR_DEBUG("chown of storage file %p to %u:%u", - src, (unsigned int)uid, (unsigned int)gid); - - return src->drv->backend->storageFileChown(src, uid, gid); -} - - -/** - * virStorageFileReportBrokenChain: - * - * @errcode: errno when accessing @src - * @src: inaccessible file in the backing chain of @parent - * @parent: root virStorageSource being checked - * - * Reports the correct error message if @src is missing in the backing chain - * for @parent. - */ -void -virStorageFileReportBrokenChain(int errcode, - virStorageSourcePtr src, - virStorageSourcePtr parent) -{ - - if (src->drv) { - unsigned int access_user = src->drv->uid; - unsigned int access_group = src->drv->gid; - - if (src == parent) { - virReportSystemError(errcode, - _("Cannot access storage file '%s' " - "(as uid:%u, gid:%u)"), - src->path, access_user, access_group); - } else { - virReportSystemError(errcode, - _("Cannot access backing file '%s' " - "of storage file '%s' (as uid:%u, gid:%u)"), - src->path, parent->path, access_user, access_group); - } - } else { - if (src == parent) { - virReportSystemError(errcode, - _("Cannot access storage file '%s'"), - src->path); - } else { - virReportSystemError(errcode, - _("Cannot access backing file '%s' " - "of storage file '%s'"), - src->path, parent->path); - } - } -} - - -/* Recursive workhorse for virStorageFileGetMetadata. */ -static int -virStorageFileGetMetadataRecurse(virStorageSourcePtr src, - virStorageSourcePtr parent, - uid_t uid, gid_t gid, - bool allow_probe, - bool report_broken, - virHashTablePtr cycle, - unsigned int depth) -{ - int ret = -1; - const char *uniqueName; - char *buf = NULL; - ssize_t headerLen; - virStorageSourcePtr backingStore = NULL; - int backingFormat; - - VIR_DEBUG("path=%s format=%d uid=%u gid=%u probe=%d", - src->path, src->format, - (unsigned int)uid, (unsigned int)gid, allow_probe); - - /* exit if we can't load information about the current image */ - if (!virStorageFileSupportsBackingChainTraversal(src)) - return 0; - - if (virStorageFileInitAs(src, uid, gid) < 0) - return -1; - - if (virStorageFileAccess(src, F_OK) < 0) { - virStorageFileReportBrokenChain(errno, src, parent); - goto cleanup; - } - - if (!(uniqueName = virStorageFileGetUniqueIdentifier(src))) - goto cleanup; - - if (virHashLookup(cycle, uniqueName)) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("backing store for %s (%s) is self-referential"), - src->path, uniqueName); - goto cleanup; - } - - if (virHashAddEntry(cycle, uniqueName, (void *)1) < 0) - goto cleanup; - - if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, - &buf)) < 0) - goto cleanup; - - if (virStorageFileGetMetadataInternal(src, buf, headerLen, - &backingFormat) < 0) - goto cleanup; - - if (src->backingStoreRaw) { - if (!(backingStore = virStorageSourceNewFromBacking(src))) - goto cleanup; - - if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) - backingStore->format = VIR_STORAGE_FILE_RAW; - else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) - backingStore->format = VIR_STORAGE_FILE_AUTO; - else - backingStore->format = backingFormat; - - if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, - uid, gid, - allow_probe, report_broken, - cycle, depth + 1)) < 0) { - if (report_broken) - goto cleanup; - - /* if we fail somewhere midway, just accept and return a - * broken chain */ - ret = 0; - goto cleanup; - } - } else { - /* add terminator */ - if (VIR_ALLOC(backingStore) < 0) - goto cleanup; - } - - src->backingStore = backingStore; - backingStore = NULL; - ret = 0; - - cleanup: - if (virStorageSourceHasBacking(src)) - src->backingStore->id = depth; - VIR_FREE(buf); - virStorageFileDeinit(src); - virStorageSourceFree(backingStore); - return ret; -} - - -/** - * virStorageFileGetMetadata: - * - * Extract metadata about the storage volume with the specified - * image format. If image format is VIR_STORAGE_FILE_AUTO, it - * will probe to automatically identify the format. Recurses through - * the entire chain. - * - * Open files using UID and GID (or pass -1 for the current user/group). - * Treat any backing files without explicit type as raw, unless ALLOW_PROBE. - * - * Callers are advised never to use VIR_STORAGE_FILE_AUTO as a - * format, since a malicious guest can turn a raw file into any - * other non-raw format at will. - * - * If @report_broken is true, the whole function fails with a possibly sane - * error instead of just returning a broken chain. - * - * Caller MUST free result after use via virStorageSourceFree. - */ -int -virStorageFileGetMetadata(virStorageSourcePtr src, - uid_t uid, gid_t gid, - bool allow_probe, - bool report_broken) -{ - VIR_DEBUG("path=%s format=%d uid=%u gid=%u probe=%d, report_broken=%d", - src->path, src->format, (unsigned int)uid, (unsigned int)gid, - allow_probe, report_broken); - - virHashTablePtr cycle = NULL; - virStorageType actualType = virStorageSourceGetActualType(src); - int ret = -1; - - if (!(cycle = virHashCreate(5, NULL))) - return -1; - - if (src->format <= VIR_STORAGE_FILE_NONE) { - if (actualType == VIR_STORAGE_TYPE_DIR) - src->format = VIR_STORAGE_FILE_DIR; - else if (allow_probe) - src->format = VIR_STORAGE_FILE_AUTO; - else - src->format = VIR_STORAGE_FILE_RAW; - } - - ret = virStorageFileGetMetadataRecurse(src, src, uid, gid, - allow_probe, report_broken, cycle, 1); - - virHashFree(cycle); - return ret; -} - - -/** - * virStorageFileGetBackingStoreStr: - * @src: storage object - * - * Extracts the backing store string as stored in the storage volume described - * by @src and returns it to the user. Caller is responsible for freeing it. - * In case when the string can't be retrieved or does not exist NULL is - * returned. - */ -char * -virStorageFileGetBackingStoreStr(virStorageSourcePtr src) -{ - virStorageSourcePtr tmp = NULL; - char *buf = NULL; - ssize_t headerLen; - char *ret = NULL; - - /* exit if we can't load information about the current image */ - if (!virStorageFileSupportsBackingChainTraversal(src)) - return NULL; - - if (virStorageFileAccess(src, F_OK) < 0) - return NULL; - - if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, - &buf)) < 0) - return NULL; - - if (!(tmp = virStorageSourceCopy(src, false))) - goto cleanup; - - if (virStorageFileGetMetadataInternal(tmp, buf, headerLen, NULL) < 0) - goto cleanup; - - VIR_STEAL_PTR(ret, tmp->backingStoreRaw); - - cleanup: - VIR_FREE(buf); - virStorageSourceFree(tmp); - - return ret; -} diff --git a/src/storage/storage_source.h b/src/storage/storage_source.h deleted file mode 100644 index 0640c138ed..0000000000 --- a/src/storage/storage_source.h +++ /dev/null @@ -1,59 +0,0 @@ -/* - * storage_source.h: Storage source accessors to real storaget - * - * 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_SOURCE_H__ -# define __VIR_STORAGE_SOURCE_H__ - -# include <sys/stat.h> - -# include "virstoragefile.h" - -int virStorageFileInit(virStorageSourcePtr src); -int virStorageFileInitAs(virStorageSourcePtr src, - uid_t uid, gid_t gid); -void virStorageFileDeinit(virStorageSourcePtr src); - -int virStorageFileCreate(virStorageSourcePtr src); -int virStorageFileUnlink(virStorageSourcePtr src); -int virStorageFileStat(virStorageSourcePtr src, - struct stat *stat); -ssize_t virStorageFileRead(virStorageSourcePtr src, - size_t offset, - size_t len, - char **buf); -const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); -int virStorageFileAccess(virStorageSourcePtr src, int mode); -int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid); - -bool virStorageFileSupportsSecurityDriver(const virStorageSource *src); -bool virStorageFileSupportsAccess(const virStorageSource *src); - -int virStorageFileGetMetadata(virStorageSourcePtr src, - uid_t uid, gid_t gid, - bool allow_probe, - bool report_broken) - ATTRIBUTE_NONNULL(1); - -char *virStorageFileGetBackingStoreStr(virStorageSourcePtr src) - ATTRIBUTE_NONNULL(1); - -void virStorageFileReportBrokenChain(int errcode, - virStorageSourcePtr src, - virStorageSourcePtr parent); - -#endif /* __VIR_STORAGE_SOURCE_H__ */ diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 5f661c956c..7f878039ba 100644 --- a/src/util/virstoragefile.c +++ b/src/util/virstoragefile.c @@ -22,7 +22,7 @@ */ #include <config.h> -#include "virstoragefile.h" +#include "virstoragefilebackend.h" #include <unistd.h> #include <fcntl.h> @@ -4103,3 +4103,610 @@ virStorageSourcePrivateDataFormatRelPath(virStorageSourcePtr src, return 0; } + +static bool +virStorageFileIsInitialized(const virStorageSource *src) +{ + return src && src->drv; +} + + +static virStorageFileBackendPtr +virStorageFileGetBackendForSupportCheck(const virStorageSource *src) +{ + int actualType; + + if (!src) + return NULL; + + if (src->drv) + return src->drv->backend; + + actualType = virStorageSourceGetActualType(src); + + return virStorageFileBackendForTypeInternal(actualType, src->protocol, false); +} + + +static bool +virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) +{ + virStorageFileBackendPtr backend; + + if (!(backend = virStorageFileGetBackendForSupportCheck(src))) + return false; + + return backend->storageFileGetUniqueIdentifier && + backend->storageFileRead && + backend->storageFileAccess; +} + + +/** + * virStorageFileSupportsSecurityDriver: + * + * @src: a storage file structure + * + * Check if a storage file supports operations needed by the security + * driver to perform labelling + */ +bool +virStorageFileSupportsSecurityDriver(const virStorageSource *src) +{ + virStorageFileBackendPtr backend; + + if (!(backend = virStorageFileGetBackendForSupportCheck(src))) + return false; + + return !!backend->storageFileChown; +} + + +/** + * virStorageFileSupportsAccess: + * + * @src: a storage file structure + * + * Check if a storage file supports checking if the storage source is accessible + * for the given vm. + */ +bool +virStorageFileSupportsAccess(const virStorageSource *src) +{ + virStorageFileBackendPtr backend; + + if (!(backend = virStorageFileGetBackendForSupportCheck(src))) + return false; + + return !!backend->storageFileAccess; +} + + +void +virStorageFileDeinit(virStorageSourcePtr src) +{ + if (!virStorageFileIsInitialized(src)) + return; + + if (src->drv->backend && + src->drv->backend->backendDeinit) + src->drv->backend->backendDeinit(src); + + VIR_FREE(src->drv); +} + + +/** + * virStorageFileInitAs: + * + * @src: storage source definition + * @uid: uid used to access the file, or -1 for current uid + * @gid: gid used to access the file, or -1 for current gid + * + * Initialize a storage source to be used with storage driver. Use the provided + * uid and gid if possible for the operations. + * + * Returns 0 if the storage file was successfully initialized, -1 if the + * initialization failed. Libvirt error is reported. + */ +int +virStorageFileInitAs(virStorageSourcePtr src, + uid_t uid, gid_t gid) +{ + int actualType = virStorageSourceGetActualType(src); + if (VIR_ALLOC(src->drv) < 0) + return -1; + + if (uid == (uid_t) -1) + src->drv->uid = geteuid(); + else + src->drv->uid = uid; + + if (gid == (gid_t) -1) + src->drv->gid = getegid(); + else + src->drv->gid = gid; + + if (!(src->drv->backend = virStorageFileBackendForType(actualType, + src->protocol))) + goto error; + + if (src->drv->backend->backendInit && + src->drv->backend->backendInit(src) < 0) + goto error; + + return 0; + + error: + VIR_FREE(src->drv); + return -1; +} + + +/** + * virStorageFileInit: + * + * See virStorageFileInitAs. The file is initialized to be accessed by the + * current user. + */ +int +virStorageFileInit(virStorageSourcePtr src) +{ + return virStorageFileInitAs(src, -1, -1); +} + + +/** + * virStorageFileCreate: Creates an empty storage file via storage driver + * + * @src: file structure pointing to the file + * + * Returns 0 on success, -2 if the function isn't supported by the backend, + * -1 on other failure. Errno is set in case of failure. + */ +int +virStorageFileCreate(virStorageSourcePtr src) +{ + int ret; + + if (!virStorageFileIsInitialized(src) || + !src->drv->backend->storageFileCreate) { + errno = ENOSYS; + return -2; + } + + ret = src->drv->backend->storageFileCreate(src); + + VIR_DEBUG("created storage file %p: ret=%d, errno=%d", + src, ret, errno); + + return ret; +} + + +/** + * virStorageFileUnlink: Unlink storage file via storage driver + * + * @src: file structure pointing to the file + * + * Unlinks the file described by the @file structure. + * + * Returns 0 on success, -2 if the function isn't supported by the backend, + * -1 on other failure. Errno is set in case of failure. + */ +int +virStorageFileUnlink(virStorageSourcePtr src) +{ + int ret; + + if (!virStorageFileIsInitialized(src) || + !src->drv->backend->storageFileUnlink) { + errno = ENOSYS; + return -2; + } + + ret = src->drv->backend->storageFileUnlink(src); + + VIR_DEBUG("unlinked storage file %p: ret=%d, errno=%d", + src, ret, errno); + + return ret; +} + + +/** + * virStorageFileStat: returns stat struct of a file via storage driver + * + * @src: file structure pointing to the file + * @stat: stat structure to return data + * + * Returns 0 on success, -2 if the function isn't supported by the backend, + * -1 on other failure. Errno is set in case of failure. +*/ +int +virStorageFileStat(virStorageSourcePtr src, + struct stat *st) +{ + int ret; + + if (!virStorageFileIsInitialized(src) || + !src->drv->backend->storageFileStat) { + errno = ENOSYS; + return -2; + } + + ret = src->drv->backend->storageFileStat(src, st); + + VIR_DEBUG("stat of storage file %p: ret=%d, errno=%d", + src, ret, errno); + + return ret; +} + + +/** + * virStorageFileRead: read bytes from a file into a buffer + * + * @src: file structure pointing to the file + * @offset: number of bytes to skip in the storage file + * @len: maximum number of bytes read from the storage file + * @buf: buffer to read the data into. (buffer shall be freed by caller) + * + * Returns the count of bytes read on success and -1 on failure, -2 if the + * function isn't supported by the backend. + * Libvirt error is reported on failure. + */ +ssize_t +virStorageFileRead(virStorageSourcePtr src, + size_t offset, + size_t len, + char **buf) +{ + ssize_t ret; + + if (!virStorageFileIsInitialized(src)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("storage file backend not initialized")); + 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)); + return -2; + } + + ret = src->drv->backend->storageFileRead(src, offset, len, buf); + + VIR_DEBUG("read '%zd' bytes from storage '%p' starting at offset '%zu'", + ret, src, offset); + + return ret; +} + + +/* + * virStorageFileGetUniqueIdentifier: Get a unique string describing the volume + * + * @src: file structure pointing to the file + * + * Returns a string uniquely describing a single volume (canonical path). + * The string shall not be freed and is valid until the storage file is + * deinitialized. Returns NULL on error and sets a libvirt error code */ +const char * +virStorageFileGetUniqueIdentifier(virStorageSourcePtr src) +{ + if (!virStorageFileIsInitialized(src)) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("storage file backend not initialized")); + return NULL; + } + + if (!src->drv->backend->storageFileGetUniqueIdentifier) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unique storage file identifier not implemented for " + "storage type %s (protocol: %s)'"), + virStorageTypeToString(src->type), + virStorageNetProtocolTypeToString(src->protocol)); + return NULL; + } + + return src->drv->backend->storageFileGetUniqueIdentifier(src); +} + + +/** + * virStorageFileAccess: Check accessibility of a storage file + * + * @src: storage file to check access permissions + * @mode: accessibility check options (see man 2 access) + * + * Returns 0 on success, -1 on error and sets errno. No libvirt + * error is reported. Returns -2 if the operation isn't supported + * by libvirt storage backend. + */ +int +virStorageFileAccess(virStorageSourcePtr src, + int mode) +{ + if (!virStorageFileIsInitialized(src) || + !src->drv->backend->storageFileAccess) { + errno = ENOSYS; + return -2; + } + + return src->drv->backend->storageFileAccess(src, mode); +} + + +/** + * virStorageFileChown: Change owner of a storage file + * + * @src: storage file to change owner of + * @uid: new owner id + * @gid: new group id + * + * Returns 0 on success, -1 on error and sets errno. No libvirt + * error is reported. Returns -2 if the operation isn't supported + * by libvirt storage backend. + */ +int +virStorageFileChown(const virStorageSource *src, + uid_t uid, + gid_t gid) +{ + if (!virStorageFileIsInitialized(src) || + !src->drv->backend->storageFileChown) { + errno = ENOSYS; + return -2; + } + + VIR_DEBUG("chown of storage file %p to %u:%u", + src, (unsigned int)uid, (unsigned int)gid); + + return src->drv->backend->storageFileChown(src, uid, gid); +} + + +/** + * virStorageFileReportBrokenChain: + * + * @errcode: errno when accessing @src + * @src: inaccessible file in the backing chain of @parent + * @parent: root virStorageSource being checked + * + * Reports the correct error message if @src is missing in the backing chain + * for @parent. + */ +void +virStorageFileReportBrokenChain(int errcode, + virStorageSourcePtr src, + virStorageSourcePtr parent) +{ + + if (src->drv) { + unsigned int access_user = src->drv->uid; + unsigned int access_group = src->drv->gid; + + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s' " + "(as uid:%u, gid:%u)"), + src->path, access_user, access_group); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s' (as uid:%u, gid:%u)"), + src->path, parent->path, access_user, access_group); + } + } else { + if (src == parent) { + virReportSystemError(errcode, + _("Cannot access storage file '%s'"), + src->path); + } else { + virReportSystemError(errcode, + _("Cannot access backing file '%s' " + "of storage file '%s'"), + src->path, parent->path); + } + } +} + + +/* Recursive workhorse for virStorageFileGetMetadata. */ +static int +virStorageFileGetMetadataRecurse(virStorageSourcePtr src, + virStorageSourcePtr parent, + uid_t uid, gid_t gid, + bool allow_probe, + bool report_broken, + virHashTablePtr cycle, + unsigned int depth) +{ + int ret = -1; + const char *uniqueName; + char *buf = NULL; + ssize_t headerLen; + virStorageSourcePtr backingStore = NULL; + int backingFormat; + + VIR_DEBUG("path=%s format=%d uid=%u gid=%u probe=%d", + src->path, src->format, + (unsigned int)uid, (unsigned int)gid, allow_probe); + + /* exit if we can't load information about the current image */ + if (!virStorageFileSupportsBackingChainTraversal(src)) + return 0; + + if (virStorageFileInitAs(src, uid, gid) < 0) + return -1; + + if (virStorageFileAccess(src, F_OK) < 0) { + virStorageFileReportBrokenChain(errno, src, parent); + goto cleanup; + } + + if (!(uniqueName = virStorageFileGetUniqueIdentifier(src))) + goto cleanup; + + if (virHashLookup(cycle, uniqueName)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("backing store for %s (%s) is self-referential"), + src->path, uniqueName); + goto cleanup; + } + + if (virHashAddEntry(cycle, uniqueName, (void *)1) < 0) + goto cleanup; + + if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, + &buf)) < 0) + goto cleanup; + + if (virStorageFileGetMetadataInternal(src, buf, headerLen, + &backingFormat) < 0) + goto cleanup; + + if (src->backingStoreRaw) { + if (!(backingStore = virStorageSourceNewFromBacking(src))) + goto cleanup; + + if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe) + backingStore->format = VIR_STORAGE_FILE_RAW; + else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE) + backingStore->format = VIR_STORAGE_FILE_AUTO; + else + backingStore->format = backingFormat; + + if ((ret = virStorageFileGetMetadataRecurse(backingStore, parent, + uid, gid, + allow_probe, report_broken, + cycle, depth + 1)) < 0) { + if (report_broken) + goto cleanup; + + /* if we fail somewhere midway, just accept and return a + * broken chain */ + ret = 0; + goto cleanup; + } + } else { + /* add terminator */ + if (VIR_ALLOC(backingStore) < 0) + goto cleanup; + } + + src->backingStore = backingStore; + backingStore = NULL; + ret = 0; + + cleanup: + if (virStorageSourceHasBacking(src)) + src->backingStore->id = depth; + VIR_FREE(buf); + virStorageFileDeinit(src); + virStorageSourceFree(backingStore); + return ret; +} + + +/** + * virStorageFileGetMetadata: + * + * Extract metadata about the storage volume with the specified + * image format. If image format is VIR_STORAGE_FILE_AUTO, it + * will probe to automatically identify the format. Recurses through + * the entire chain. + * + * Open files using UID and GID (or pass -1 for the current user/group). + * Treat any backing files without explicit type as raw, unless ALLOW_PROBE. + * + * Callers are advised never to use VIR_STORAGE_FILE_AUTO as a + * format, since a malicious guest can turn a raw file into any + * other non-raw format at will. + * + * If @report_broken is true, the whole function fails with a possibly sane + * error instead of just returning a broken chain. + * + * Caller MUST free result after use via virStorageSourceFree. + */ +int +virStorageFileGetMetadata(virStorageSourcePtr src, + uid_t uid, gid_t gid, + bool allow_probe, + bool report_broken) +{ + VIR_DEBUG("path=%s format=%d uid=%u gid=%u probe=%d, report_broken=%d", + src->path, src->format, (unsigned int)uid, (unsigned int)gid, + allow_probe, report_broken); + + virHashTablePtr cycle = NULL; + virStorageType actualType = virStorageSourceGetActualType(src); + int ret = -1; + + if (!(cycle = virHashCreate(5, NULL))) + return -1; + + if (src->format <= VIR_STORAGE_FILE_NONE) { + if (actualType == VIR_STORAGE_TYPE_DIR) + src->format = VIR_STORAGE_FILE_DIR; + else if (allow_probe) + src->format = VIR_STORAGE_FILE_AUTO; + else + src->format = VIR_STORAGE_FILE_RAW; + } + + ret = virStorageFileGetMetadataRecurse(src, src, uid, gid, + allow_probe, report_broken, cycle, 1); + + virHashFree(cycle); + return ret; +} + + +/** + * virStorageFileGetBackingStoreStr: + * @src: storage object + * + * Extracts the backing store string as stored in the storage volume described + * by @src and returns it to the user. Caller is responsible for freeing it. + * In case when the string can't be retrieved or does not exist NULL is + * returned. + */ +char * +virStorageFileGetBackingStoreStr(virStorageSourcePtr src) +{ + virStorageSourcePtr tmp = NULL; + char *buf = NULL; + ssize_t headerLen; + char *ret = NULL; + + /* exit if we can't load information about the current image */ + if (!virStorageFileSupportsBackingChainTraversal(src)) + return NULL; + + if (virStorageFileAccess(src, F_OK) < 0) + return NULL; + + if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, + &buf)) < 0) + return NULL; + + if (!(tmp = virStorageSourceCopy(src, false))) + goto cleanup; + + if (virStorageFileGetMetadataInternal(tmp, buf, headerLen, NULL) < 0) + goto cleanup; + + VIR_STEAL_PTR(ret, tmp->backingStoreRaw); + + cleanup: + VIR_FREE(buf); + virStorageSourceFree(tmp); + + return ret; +} diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index ecd806c93f..9e8afa4932 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -448,5 +448,37 @@ int virStorageSourcePrivateDataFormatRelPath(virStorageSourcePtr src, virBufferPtr buf); +int virStorageFileInit(virStorageSourcePtr src); +int virStorageFileInitAs(virStorageSourcePtr src, + uid_t uid, gid_t gid); +void virStorageFileDeinit(virStorageSourcePtr src); + +int virStorageFileCreate(virStorageSourcePtr src); +int virStorageFileUnlink(virStorageSourcePtr src); +int virStorageFileStat(virStorageSourcePtr src, + struct stat *stat); +ssize_t virStorageFileRead(virStorageSourcePtr src, + size_t offset, + size_t len, + char **buf); +const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src); +int virStorageFileAccess(virStorageSourcePtr src, int mode); +int virStorageFileChown(const virStorageSource *src, uid_t uid, gid_t gid); + +bool virStorageFileSupportsSecurityDriver(const virStorageSource *src); +bool virStorageFileSupportsAccess(const virStorageSource *src); + +int virStorageFileGetMetadata(virStorageSourcePtr src, + uid_t uid, gid_t gid, + bool allow_probe, + bool report_broken) + ATTRIBUTE_NONNULL(1); + +char *virStorageFileGetBackingStoreStr(virStorageSourcePtr src) + ATTRIBUTE_NONNULL(1); + +void virStorageFileReportBrokenChain(int errcode, + virStorageSourcePtr src, + virStorageSourcePtr parent); #endif /* __VIR_STORAGE_FILE_H__ */ diff --git a/src/storage/storage_source_backend.c b/src/util/virstoragefilebackend.c similarity index 96% rename from src/storage/storage_source_backend.c rename to src/util/virstoragefilebackend.c index e093c04989..df86ee39c2 100644 --- a/src/storage/storage_source_backend.c +++ b/src/util/virstoragefilebackend.c @@ -1,5 +1,5 @@ /* - * storage_source_backend.c: internal storage source backend contract + * virstoragefilebackend.c: internal storage source backend contract * * Copyright (C) 2007-2018 Red Hat, Inc. * Copyright (C) 2007-2008 Daniel P. Berrange @@ -30,7 +30,7 @@ #include "virerror.h" #include "viralloc.h" #include "internal.h" -#include "storage_source_backend.h" +#include "virstoragefilebackend.h" #include "virlog.h" #include "virfile.h" #include "configmake.h" diff --git a/src/storage/storage_source_backend.h b/src/util/virstoragefilebackend.h similarity index 94% rename from src/storage/storage_source_backend.h rename to src/util/virstoragefilebackend.h index 3af2a5f380..6cd51750ee 100644 --- a/src/storage/storage_source_backend.h +++ b/src/util/virstoragefilebackend.h @@ -1,5 +1,5 @@ /* - * storage_source_backend.h: internal storage source backend contract + * virstoragefilebackend.h: internal storage source backend contract * * Copyright (C) 2007-2018 Red Hat, Inc. * @@ -18,8 +18,8 @@ * <http://www.gnu.org/licenses/>. */ -#ifndef __VIR_STORAGE_SOURCE_BACKEND_H__ -# define __VIR_STORAGE_SOURCE_BACKEND_H__ +#ifndef __VIR_STORAGE_FILE_BACKEND_H__ +# define __VIR_STORAGE_FILE_BACKEND_H__ # include <sys/stat.h> @@ -101,4 +101,4 @@ struct _virStorageFileBackend { int virStorageFileBackendRegister(virStorageFileBackendPtr backend); -#endif /* __VIR_STORAGE_SOURCE_BACKEND_H__ */ +#endif /* __VIR_STORAGE_FILE_BACKEND_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index 1dc7608cee..6eed7134ed 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -32,7 +32,6 @@ #include "dirname.h" #include "storage/storage_driver.h" -#include "storage/storage_source.h" #define VIR_FROM_THIS VIR_FROM_NONE -- 2.14.3
The libvirt_driver_remote.la static library is linked into the libvirt.so dynamic library, providing both the generic RPC layer code and the remote protocol client driver. The libvirtd daemon the itself links to libvirt_driver_remote.la, in order to get access to the generic RPC layer code and the XDR functions for the remote driver. This means we get multiple copies of the same code in libvirtd, one direct and one indirect via libvirt.so. The same mistake affects the lockd plugin. The libvirtd daemon should instead just link aganist the generic RPC layer code that's in libvirt.so. This is easily doable if we add exports for the few symbols we've previously missed, and wildcard export xdr_* to expose the auto-generated XDR marshallers. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- daemon/Makefile.am | 1 - src/Makefile.am | 12 +++++------- src/libvirt_remote.syms | 11 ++++++++++- 3 files changed, 15 insertions(+), 9 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index efd5ff19f5..9c1dfcfac6 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -176,7 +176,6 @@ libvirtd_LDADD += \ ../src/libvirt_driver_admin.la \ ../src/libvirt-lxc.la \ ../src/libvirt-qemu.la \ - ../src/libvirt_driver_remote.la \ $(NULL) libvirtd_LDADD += ../src/libvirt.la diff --git a/src/Makefile.am b/src/Makefile.am index 54ef81d1ab..5d84396644 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1316,16 +1316,11 @@ if WITH_REMOTE noinst_LTLIBRARIES += libvirt_driver_remote.la libvirt_la_BUILT_LIBADD += libvirt_driver_remote.la libvirt_driver_remote_la_CFLAGS = \ - $(GNUTLS_CFLAGS) \ $(XDR_CFLAGS) \ -I$(srcdir)/conf \ -I$(srcdir)/rpc \ $(AM_CFLAGS) libvirt_driver_remote_la_LDFLAGS = $(AM_LDFLAGS) -libvirt_driver_remote_la_LIBADD = $(GNUTLS_LIBS) \ - libvirt-net-rpc-client.la \ - libvirt-net-rpc-server.la \ - libvirt-net-rpc.la libvirt_driver_remote_la_SOURCES = $(REMOTE_DRIVER_SOURCES) BUILT_SOURCES += $(REMOTE_DRIVER_GENERATED) @@ -2602,8 +2597,6 @@ lockd_la_CFLAGS = -I$(srcdir)/conf \ $(AM_CFLAGS) lockd_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) lockd_la_LIBADD = ../gnulib/lib/libgnu.la \ - libvirt-net-rpc.la \ - libvirt-net-rpc-client.la \ $(NULL) augeas_DATA += locking/libvirt_lockd.aug if WITH_DTRACE_PROBES @@ -2919,6 +2912,11 @@ noinst_LTLIBRARIES += \ libvirt-net-rpc-server.la \ libvirt-net-rpc-client.la +libvirt_la_BUILT_LIBADD += \ + libvirt-net-rpc.la \ + libvirt-net-rpc-server.la \ + libvirt-net-rpc-client.la + EXTRA_DIST += \ dtrace2systemtap.pl \ rpc/gendispatch.pl \ diff --git a/src/libvirt_remote.syms b/src/libvirt_remote.syms index b75cbb99b2..736848273a 100644 --- a/src/libvirt_remote.syms +++ b/src/libvirt_remote.syms @@ -5,6 +5,9 @@ # Keep this file sorted by header name, then by symbols with each header. # +# Generated files +xdr_*; + # rpc/virnetclient.h virNetClientAddProgram; virNetClientAddStream; @@ -80,6 +83,7 @@ virNetDaemonUpdateServices; # rpc/virnetmessage.h +virNetMessageAddFD; virNetMessageClear; virNetMessageClearPayload; virNetMessageDecodeHeader; @@ -96,7 +100,6 @@ virNetMessageNew; virNetMessageQueuePush; virNetMessageQueueServe; virNetMessageSaveError; -xdr_virNetMessageError; # rpc/virnetserver.h @@ -104,12 +107,14 @@ virNetServerAddClient; virNetServerAddProgram; virNetServerAddService; virNetServerClose; +virNetServerGetClient; virNetServerGetClients; virNetServerGetCurrentClients; virNetServerGetCurrentUnauthClients; virNetServerGetMaxClients; virNetServerGetMaxUnauthClients; virNetServerGetName; +virNetServerGetThreadPoolParameters; virNetServerHasClients; virNetServerNew; virNetServerNewPostExecRestart; @@ -117,6 +122,8 @@ virNetServerNextClientID; virNetServerPreExecRestart; virNetServerProcessClients; virNetServerSetClientAuthenticated; +virNetServerSetClientLimits; +virNetServerSetThreadPoolParameters; virNetServerStart; virNetServerUpdateServices; @@ -128,11 +135,13 @@ virNetServerClientCloseLocked; virNetServerClientDelayedClose; virNetServerClientGetAuth; virNetServerClientGetFD; +virNetServerClientGetID; virNetServerClientGetIdentity; virNetServerClientGetInfo; virNetServerClientGetPrivateData; virNetServerClientGetReadonly; virNetServerClientGetSELinuxContext; +virNetServerClientGetTimestamp; virNetServerClientGetTransport; virNetServerClientGetUNIXIdentity; virNetServerClientImmediateClose; -- 2.14.3
Rather than static linking in various of the helper libraries to libvirt_lxc, just link against the main libvirt.so. This is more memory and time efficient because it will already be cached in memory and sharable between processes. CAPNG flags need adding because the LXC code directly calls various libcapng APIs and no longer inherits the CAPNG flags via the statically linked .a libs. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/Makefile.am | 10 +++++----- src/libvirt_private.syms | 2 ++ 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 5d84396644..7f9961fe55 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -3113,14 +3113,12 @@ libvirt_lxc_SOURCES = \ libvirt_lxc_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ + $(CAPNG_LIBS) \ + $(LIBXML_LIBS) \ $(NULL) libvirt_lxc_LDADD = \ $(FUSE_LIBS) \ - libvirt-net-rpc-server.la \ - libvirt-net-rpc.la \ - libvirt_security_manager.la \ - libvirt_conf.la \ - libvirt_util.la \ + libvirt.la \ ../gnulib/lib/libgnu.la if WITH_DTRACE_PROBES libvirt_lxc_LDADD += libvirt_probes.lo @@ -3130,6 +3128,8 @@ libvirt_lxc_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) \ $(PIE_CFLAGS) \ + $(CAPNG_CFLAGS) \ + $(LIBXML_CFLAGS) \ $(LIBNL_CFLAGS) \ $(FUSE_CFLAGS) \ $(DBUS_CFLAGS) \ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 3ec510bd32..bbe97b161f 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -347,6 +347,7 @@ virDomainDiskSetSource; virDomainDiskSetType; virDomainFSDefFree; virDomainFSDefNew; +virDomainFSDriverTypeToString; virDomainFSIndexByName; virDomainFSInsert; virDomainFSRemove; @@ -470,6 +471,7 @@ virDomainObjGetOneDefState; virDomainObjGetPersistentDef; virDomainObjGetState; virDomainObjNew; +virDomainObjParseFile; virDomainObjParseNode; virDomainObjRemoveTransientDef; virDomainObjSetDefTransient; -- 2.14.3
Currently virt drivers will call directly into the network driver impl to allocate domain interface devices where type=network. This introduces a callback system to allow us to decouple the virt drivers from the network driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 54 +++++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 34 ++++++++++++++++++++++++++++ src/libvirt_private.syms | 4 ++++ src/libxl/libxl_domain.c | 5 ++--- src/libxl/libxl_driver.c | 7 +++--- src/lxc/lxc_driver.c | 5 ++--- src/lxc/lxc_process.c | 7 +++--- src/network/bridge_driver.c | 12 +++++++--- src/network/bridge_driver.h | 28 ----------------------- src/qemu/qemu_hotplug.c | 15 ++++++------- src/qemu/qemu_process.c | 6 ++--- 11 files changed, 121 insertions(+), 56 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 01d168eb87..1e3a83cf73 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28816,3 +28816,57 @@ virDomainNetTypeSharesHostView(const virDomainNetDef *net) } return false; } + +static virDomainNetAllocateActualDeviceImpl netAllocate; +static virDomainNetNotifyActualDeviceImpl netNotify; +static virDomainNetReleaseActualDeviceImpl netRelease; + +void +virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, + virDomainNetNotifyActualDeviceImpl notify, + virDomainNetReleaseActualDeviceImpl release) +{ + netAllocate = allocate; + netNotify = notify; + netRelease = release; +} + +int +virDomainNetAllocateActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + if (!netAllocate) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device allocation not available")); + return -1; + } + + return netAllocate(dom, iface); +} + +void +virDomainNetNotifyActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + if (!netNotify) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device notification not available")); + return; + } + + netNotify(dom, iface); +} + + +int +virDomainNetReleaseActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) +{ + if (!netRelease) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device release not available")); + return -1; + } + + return netRelease(dom, iface); +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 21e0045157..45461f8ef2 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3454,4 +3454,38 @@ bool virDomainDefLifecycleActionAllowed(virDomainLifecycle type, virDomainLifecycleAction action); +typedef int +(*virDomainNetAllocateActualDeviceImpl)(virDomainDefPtr dom, + virDomainNetDefPtr iface); + +typedef void +(*virDomainNetNotifyActualDeviceImpl)(virDomainDefPtr dom, + virDomainNetDefPtr iface); + +typedef int +(*virDomainNetReleaseActualDeviceImpl)(virDomainDefPtr dom, + virDomainNetDefPtr iface); + +void +virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, + virDomainNetNotifyActualDeviceImpl notify, + virDomainNetReleaseActualDeviceImpl release); + +int +virDomainNetAllocateActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +void +virDomainNetNotifyActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int +virDomainNetReleaseActualDevice(virDomainDefPtr dom, + virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + + + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index bbe97b161f..ecb90febc0 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -433,6 +433,7 @@ virDomainMemoryModelTypeToString; virDomainMemoryRemove; virDomainMemorySourceTypeFromString; virDomainMemorySourceTypeToString; +virDomainNetAllocateActualDevice; virDomainNetAppendIPAddress; virDomainNetDefClear; virDomainNetDefFormat; @@ -452,8 +453,11 @@ virDomainNetGetActualType; virDomainNetGetActualVirtPortProfile; virDomainNetGetActualVlan; virDomainNetInsert; +virDomainNetNotifyActualDevice; +virDomainNetReleaseActualDevice; virDomainNetRemove; virDomainNetRemoveHostdev; +virDomainNetSetDeviceImpl; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; virDomainNetTypeToString; diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c index 395c8a921b..feb092ba48 100644 --- a/src/libxl/libxl_domain.c +++ b/src/libxl/libxl_domain.c @@ -38,7 +38,6 @@ #include "virtime.h" #include "locking/domain_lock.h" #include "xen_common.h" -#include "network/bridge_driver.h" #define VIR_FROM_THIS VIR_FROM_LIBXL @@ -796,7 +795,7 @@ libxlDomainCleanup(libxlDriverPrivatePtr driver, /* cleanup actual device */ virDomainNetRemoveHostdev(vm->def, net); - networkReleaseActualDevice(vm->def, net); + virDomainNetReleaseActualDevice(vm->def, net); } } @@ -955,7 +954,7 @@ libxlNetworkPrepareDevices(virDomainDefPtr def) * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(def, net) < 0) + if (virDomainNetAllocateActualDevice(def, net) < 0) return -1; actualType = virDomainNetGetActualType(net); diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c index 79e29ce072..be11134fb2 100644 --- a/src/libxl/libxl_driver.c +++ b/src/libxl/libxl_driver.c @@ -59,7 +59,6 @@ #include "viraccessapicheck.h" #include "viratomic.h" #include "virhostdev.h" -#include "network/bridge_driver.h" #include "locking/domain_lock.h" #include "virnetdevtap.h" #include "cpu/cpu.h" @@ -3349,7 +3348,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(vm->def, net) < 0) + if (virDomainNetAllocateActualDevice(vm->def, net) < 0) goto cleanup; actualType = virDomainNetGetActualType(net); @@ -3399,7 +3398,7 @@ libxlDomainAttachNetDevice(libxlDriverPrivatePtr driver, vm->def->nets[vm->def->nnets++] = net; } else { virDomainNetRemoveHostdev(vm->def, net); - networkReleaseActualDevice(vm->def, net); + virDomainNetReleaseActualDevice(vm->def, net); } virObjectUnref(cfg); return ret; @@ -3822,7 +3821,7 @@ libxlDomainDetachNetDevice(libxlDriverPrivatePtr driver, cleanup: libxl_device_nic_dispose(&nic); if (!ret) { - networkReleaseActualDevice(vm->def, detach); + virDomainNetReleaseActualDevice(vm->def, detach); virDomainNetRemove(vm->def, detachidx); } virObjectUnref(cfg); diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index b3447100fc..961baa344c 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -67,7 +67,6 @@ #include "domain_audit.h" #include "domain_nwfilter.h" #include "nwfilter_conf.h" -#include "network/bridge_driver.h" #include "virinitctl.h" #include "virnetdev.h" #include "virnetdevtap.h" @@ -3944,7 +3943,7 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(vm->def, net) < 0) + if (virDomainNetAllocateActualDevice(vm->def, net) < 0) return -1; actualType = virDomainNetGetActualType(net); @@ -4468,7 +4467,7 @@ lxcDomainDetachDeviceNetLive(virDomainObjPtr vm, ret = 0; cleanup: if (!ret) { - networkReleaseActualDevice(vm->def, detach); + virDomainNetReleaseActualDevice(vm->def, detach); virDomainNetRemove(vm->def, detachidx); virDomainNetDefFree(detach); } diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index efd8a69000..bc321e360d 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -40,7 +40,6 @@ #include "virnetdevopenvswitch.h" #include "virtime.h" #include "domain_nwfilter.h" -#include "network/bridge_driver.h" #include "viralloc.h" #include "domain_audit.h" #include "virerror.h" @@ -219,7 +218,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver, iface->ifname)); ignore_value(virNetDevVethDelete(iface->ifname)); } - networkReleaseActualDevice(vm->def, iface); + virDomainNetReleaseActualDevice(vm->def, iface); } virDomainConfVMNWFilterTeardown(vm); @@ -553,7 +552,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, if (virLXCProcessValidateInterface(net) < 0) return -1; - if (networkAllocateActualDevice(def, net) < 0) + if (virDomainNetAllocateActualDevice(def, net) < 0) goto cleanup; if (VIR_EXPAND_N(*veths, *nveths, 1) < 0) @@ -635,7 +634,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, ignore_value(virNetDevOpenvswitchRemovePort( virDomainNetGetActualBridgeName(iface), iface->ifname)); - networkReleaseActualDevice(def, iface); + virDomainNetReleaseActualDevice(def, iface); } } return ret; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index b9da0569f1..4113217882 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -4374,7 +4374,7 @@ networkLogAllocation(virNetworkDefPtr netdef, * * Returns 0 on success, -1 on failure. */ -int +static int networkAllocateActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) { @@ -4796,7 +4796,7 @@ networkAllocateActualDevice(virDomainDefPtr dom, * * No return value (but does log any failures) */ -void +static void networkNotifyActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) { @@ -5013,7 +5013,7 @@ networkNotifyActualDevice(virDomainDefPtr dom, * * Returns 0 on success, -1 on failure. */ -int +static int networkReleaseActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) { @@ -5810,5 +5810,11 @@ networkRegister(void) return -1; if (virRegisterStateDriver(&networkStateDriver) < 0) return -1; + + virDomainNetSetDeviceImpl( + networkAllocateActualDevice, + networkNotifyActualDevice, + networkReleaseActualDevice); + return 0; } diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 6d9aece656..dbb4fa8086 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -35,20 +35,6 @@ int networkRegister(void); # if WITH_NETWORK -int -networkAllocateActualDevice(virDomainDefPtr dom, - virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - -void -networkNotifyActualDevice(virDomainDefPtr dom, - virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - -int -networkReleaseActualDevice(virDomainDefPtr dom, - virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); int networkGetNetworkAddress(const char *netname, @@ -78,25 +64,11 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, # else /* Define no-op replacements that don't drag in any link dependencies. */ -# define networkAllocateActualDevice(dom, iface) 0 # define networkGetActualType(iface) (iface->type) # define networkGetNetworkAddress(netname, netaddr) (-2) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 -static inline void -networkNotifyActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED, - virDomainNetDefPtr iface ATTRIBUTE_UNUSED) -{ -} - -static inline int -networkReleaseActualDevice(virDomainDefPtr dom ATTRIBUTE_UNUSED, - virDomainNetDefPtr iface ATTRIBUTE_UNUSED) -{ - return 0; -} - static inline bool networkBandwidthChangeAllowed(virDomainNetDefPtr iface ATTRIBUTE_UNUSED, virNetDevBandwidthPtr newBandwidth ATTRIBUTE_UNUSED) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 53bfe47d08..1a406b6d26 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -47,7 +47,6 @@ #include "virprocess.h" #include "qemu_cgroup.h" #include "locking/domain_lock.h" -#include "network/bridge_driver.h" #include "virnetdev.h" #include "virnetdevbridge.h" #include "virnetdevtap.h" @@ -855,7 +854,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(vm->def, net) < 0) + if (virDomainNetAllocateActualDevice(vm->def, net) < 0) goto cleanup; actualType = virDomainNetGetActualType(net); @@ -1186,7 +1185,7 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, virDomainNetRemoveHostdev(vm->def, net); - networkReleaseActualDevice(vm->def, net); + virDomainNetReleaseActualDevice(vm->def, net); } VIR_FREE(nicstr); @@ -3233,7 +3232,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, * free it if we fail for any reason */ if (newdev->type == VIR_DOMAIN_NET_TYPE_NETWORK && - networkAllocateActualDevice(vm->def, newdev) < 0) { + virDomainNetAllocateActualDevice(vm->def, newdev) < 0) { goto cleanup; } @@ -3441,7 +3440,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, /* this function doesn't work with HOSTDEV networks yet, thus * no need to change the pointer in the hostdev structure */ - networkReleaseActualDevice(vm->def, olddev); + virDomainNetReleaseActualDevice(vm->def, olddev); virDomainNetDefFree(olddev); /* move newdev into the nets list, and NULL it out from the * virDomainDeviceDef that we were given so that the caller @@ -3473,7 +3472,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, * replace the entire device object. */ if (newdev) - networkReleaseActualDevice(vm->def, newdev); + virDomainNetReleaseActualDevice(vm->def, newdev); return ret; } @@ -4059,7 +4058,7 @@ qemuDomainRemoveHostDevice(virQEMUDriverPtr driver, virDomainHostdevDefFree(hostdev); if (net) { - networkReleaseActualDevice(vm->def, net); + virDomainNetReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); } @@ -4170,7 +4169,7 @@ qemuDomainRemoveNetDevice(virQEMUDriverPtr driver, qemuDomainNetDeviceVportRemove(net); - networkReleaseActualDevice(vm->def, net); + virDomainNetReleaseActualDevice(vm->def, net); virDomainNetDefFree(net); ret = 0; diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 5a364730c8..7652eadbec 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -2922,7 +2922,7 @@ qemuProcessNotifyNets(virDomainDefPtr def) if (virDomainNetGetActualType(net) == VIR_DOMAIN_NET_TYPE_DIRECT) ignore_value(virNetDevMacVLanReserveName(net->ifname, false)); - networkNotifyActualDevice(def, net); + virDomainNetNotifyActualDevice(def, net); } } @@ -5018,7 +5018,7 @@ qemuProcessNetworkPrepareDevices(virDomainDefPtr def) * network's pool of devices, or resolve bridge device name * to the one defined in the network definition. */ - if (networkAllocateActualDevice(def, net) < 0) + if (virDomainNetAllocateActualDevice(def, net) < 0) goto cleanup; actualType = virDomainNetGetActualType(net); @@ -6605,7 +6605,7 @@ void qemuProcessStop(virQEMUDriverPtr driver, /* kick the device out of the hostdev list too */ virDomainNetRemoveHostdev(def, net); - networkReleaseActualDevice(vm->def, net); + virDomainNetReleaseActualDevice(vm->def, net); } retry: -- 2.14.3
Currently the QEMU driver will call directly into the network driver impl to modify network device bandwidth for interfaces with type=network. This introduces a callback system to allow us to decouple the QEMU driver from the network driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 22 +++++++++++++++++++++- src/libvirt_private.syms | 2 ++ src/network/bridge_driver.c | 8 +++++--- src/network/bridge_driver.h | 24 ------------------------ src/qemu/qemu_driver.c | 5 ++--- 6 files changed, 64 insertions(+), 32 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e3a83cf73..205f99618d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28820,15 +28820,22 @@ virDomainNetTypeSharesHostView(const virDomainNetDef *net) static virDomainNetAllocateActualDeviceImpl netAllocate; static virDomainNetNotifyActualDeviceImpl netNotify; static virDomainNetReleaseActualDeviceImpl netRelease; +static virDomainNetBandwidthChangeAllowedImpl netBandwidthChangeAllowed; +static virDomainNetBandwidthUpdateImpl netBandwidthUpdate; + void virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, - virDomainNetReleaseActualDeviceImpl release) + virDomainNetReleaseActualDeviceImpl release, + virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, + virDomainNetBandwidthUpdateImpl bandwidthUpdate) { netAllocate = allocate; netNotify = notify; netRelease = release; + netBandwidthChangeAllowed = bandwidthChangeAllowed; + netBandwidthUpdate = bandwidthUpdate; } int @@ -28870,3 +28877,29 @@ virDomainNetReleaseActualDevice(virDomainDefPtr dom, return netRelease(dom, iface); } + +bool +virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth) +{ + if (!netBandwidthChangeAllowed) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device release not available")); + return -1; + } + + return netBandwidthChangeAllowed(iface, newBandwidth); +} + +int +virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth) +{ + if (!netBandwidthUpdate) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device release not available")); + return -1; + } + + return netBandwidthUpdate(iface, newBandwidth); +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 45461f8ef2..32948e181c 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3466,10 +3466,21 @@ typedef int (*virDomainNetReleaseActualDeviceImpl)(virDomainDefPtr dom, virDomainNetDefPtr iface); +typedef bool +(*virDomainNetBandwidthChangeAllowedImpl)(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth); + +typedef int +(*virDomainNetBandwidthUpdateImpl)(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth); + + void virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, - virDomainNetReleaseActualDeviceImpl release); + virDomainNetReleaseActualDeviceImpl release, + virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, + virDomainNetBandwidthUpdateImpl bandwidthUpdate); int virDomainNetAllocateActualDevice(virDomainDefPtr dom, @@ -3486,6 +3497,15 @@ virDomainNetReleaseActualDevice(virDomainDefPtr dom, virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +bool +virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); + +int +virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth) + ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index ecb90febc0..45ae25e57c 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -435,6 +435,8 @@ virDomainMemorySourceTypeFromString; virDomainMemorySourceTypeToString; virDomainNetAllocateActualDevice; virDomainNetAppendIPAddress; +virDomainNetBandwidthChangeAllowed; +virDomainNetBandwidthUpdate; virDomainNetDefClear; virDomainNetDefFormat; virDomainNetDefFree; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 4113217882..ab1d82ed31 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5631,7 +5631,7 @@ networkBandwidthGenericChecks(virDomainNetDefPtr iface, } -bool +static bool networkBandwidthChangeAllowed(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth) { @@ -5662,7 +5662,7 @@ networkBandwidthChangeAllowed(virDomainNetDefPtr iface, } -int +static int networkBandwidthUpdate(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth) { @@ -5814,7 +5814,9 @@ networkRegister(void) virDomainNetSetDeviceImpl( networkAllocateActualDevice, networkNotifyActualDevice, - networkReleaseActualDevice); + networkReleaseActualDevice, + networkBandwidthChangeAllowed, + networkBandwidthUpdate); return 0; } diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index dbb4fa8086..098f9e5c5b 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -52,16 +52,6 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, dnsmasqContext *dctx, dnsmasqCapsPtr caps); -bool -networkBandwidthChangeAllowed(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - -int -networkBandwidthUpdate(virDomainNetDefPtr iface, - virNetDevBandwidthPtr newBandwidth) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - # else /* Define no-op replacements that don't drag in any link dependencies. */ # define networkGetActualType(iface) (iface->type) @@ -69,20 +59,6 @@ networkBandwidthUpdate(virDomainNetDefPtr iface, # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 -static inline bool -networkBandwidthChangeAllowed(virDomainNetDefPtr iface ATTRIBUTE_UNUSED, - virNetDevBandwidthPtr newBandwidth ATTRIBUTE_UNUSED) -{ - return true; -} - -static inline int -networkBandwidthUpdate(virDomainNetDefPtr iface ATTRIBUTE_UNUSED, - virNetDevBandwidthPtr newBandwidth ATTRIBUTE_UNUSED) -{ - return 0; -} - # endif #endif /* __VIR_NETWORK__DRIVER_H */ diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 2b8b0682d5..c9bbe0ca37 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -106,7 +106,6 @@ #include "virperf.h" #include "virnuma.h" #include "dirname.h" -#include "network/bridge_driver.h" #include "netdev_bandwidth_conf.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -11265,12 +11264,12 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, sizeof(*newBandwidth->out)); } - if (!networkBandwidthChangeAllowed(net, newBandwidth)) + if (!virDomainNetBandwidthChangeAllowed(net, newBandwidth)) goto endjob; if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, !virDomainNetTypeSharesHostView(net)) < 0 || - networkBandwidthUpdate(net, newBandwidth) < 0) { + virDomainNetBandwidthUpdate(net, newBandwidth) < 0) { ignore_value(virNetDevBandwidthSet(net->ifname, net->bandwidth, false, -- 2.14.3
On 02/05/2018 04:28 PM, Daniel P. Berrangé wrote:
Currently the QEMU driver will call directly into the network driver impl to modify network device bandwidth for interfaces with type=network. This introduces a callback system to allow us to decouple the QEMU driver from the network driver.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 35 ++++++++++++++++++++++++++++++++++- src/conf/domain_conf.h | 22 +++++++++++++++++++++- src/libvirt_private.syms | 2 ++ src/network/bridge_driver.c | 8 +++++--- src/network/bridge_driver.h | 24 ------------------------ src/qemu/qemu_driver.c | 5 ++--- 6 files changed, 64 insertions(+), 32 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 1e3a83cf73..205f99618d 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28820,15 +28820,22 @@ virDomainNetTypeSharesHostView(const virDomainNetDef *net) static virDomainNetAllocateActualDeviceImpl netAllocate; static virDomainNetNotifyActualDeviceImpl netNotify; static virDomainNetReleaseActualDeviceImpl netRelease; +static virDomainNetBandwidthChangeAllowedImpl netBandwidthChangeAllowed; +static virDomainNetBandwidthUpdateImpl netBandwidthUpdate; +
void virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, - virDomainNetReleaseActualDeviceImpl release) + virDomainNetReleaseActualDeviceImpl release, + virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, + virDomainNetBandwidthUpdateImpl bandwidthUpdate) { netAllocate = allocate; netNotify = notify; netRelease = release; + netBandwidthChangeAllowed = bandwidthChangeAllowed; + netBandwidthUpdate = bandwidthUpdate; }
int @@ -28870,3 +28877,29 @@ virDomainNetReleaseActualDevice(virDomainDefPtr dom,
return netRelease(dom, iface); } + +bool +virDomainNetBandwidthChangeAllowed(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth) +{ + if (!netBandwidthChangeAllowed) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device release not available"));
"Network device bandwidth change not available" or something among those lines.
+ return -1; + } + + return netBandwidthChangeAllowed(iface, newBandwidth); +}> + +int +virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, + virNetDevBandwidthPtr newBandwidth) +{ + if (!netBandwidthUpdate) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device release not available"));
Same here.
+ return -1; + }
Michal
The QEMU driver calls into the network driver to get the first IP address of the network. This information is readily available via the formal public API by fetching the XML doc and then parsing it. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/network/bridge_driver.c | 98 ----------------------------------------- src/network/bridge_driver.h | 6 --- src/qemu/qemu_process.c | 104 ++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 96 insertions(+), 112 deletions(-) diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ab1d82ed31..9c0b3c0895 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5146,104 +5146,6 @@ networkReleaseActualDevice(virDomainDefPtr dom, } -/* - * networkGetNetworkAddress: - * @netname: the name of a network - * @netaddr: string representation of IP address for that network. - * - * Attempt to return an IP address associated with the named - * network. If a libvirt virtual network, that will be provided in the - * configuration. For host bridge and direct (macvtap) networks, we - * must do an ioctl to learn the address. - * - * Note: This function returns the first IP address it finds. It might - * be useful if it was more flexible, but the current use (getting a - * listen address for qemu's vnc/spice graphics server) can only use a - * single address anyway. - * - * Returns 0 on success, and puts a string (which must be free'd by - * the caller) into *netaddr. Returns -1 on failure or -2 if - * completely unsupported. - */ -int -networkGetNetworkAddress(const char *netname, - char **netaddr) -{ - virNetworkDriverStatePtr driver = networkGetDriver(); - int ret = -1; - virNetworkObjPtr obj; - virNetworkDefPtr netdef; - virNetworkIPDefPtr ipdef; - virSocketAddr addr; - virSocketAddrPtr addrptr = NULL; - char *dev_name = NULL; - - *netaddr = NULL; - obj = virNetworkObjFindByName(driver->networks, netname); - if (!obj) { - virReportError(VIR_ERR_NO_NETWORK, - _("no network with matching name '%s'"), - netname); - goto cleanup; - } - netdef = virNetworkObjGetDef(obj); - - switch (netdef->forward.type) { - case VIR_NETWORK_FORWARD_NONE: - case VIR_NETWORK_FORWARD_NAT: - case VIR_NETWORK_FORWARD_ROUTE: - case VIR_NETWORK_FORWARD_OPEN: - ipdef = virNetworkDefGetIPByIndex(netdef, AF_UNSPEC, 0); - if (!ipdef) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' doesn't have an IP address"), - netdef->name); - goto cleanup; - } - addrptr = &ipdef->address; - break; - - case VIR_NETWORK_FORWARD_BRIDGE: - if ((dev_name = netdef->bridge)) - break; - /* - * fall through if netdef->bridge wasn't set, since that is - * macvtap bridge mode network. - */ - ATTRIBUTE_FALLTHROUGH; - - case VIR_NETWORK_FORWARD_PRIVATE: - case VIR_NETWORK_FORWARD_VEPA: - case VIR_NETWORK_FORWARD_PASSTHROUGH: - if ((netdef->forward.nifs > 0) && netdef->forward.ifs) - dev_name = netdef->forward.ifs[0].device.dev; - - if (!dev_name) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("network '%s' has no associated interface or bridge"), - netdef->name); - goto cleanup; - } - break; - } - - if (dev_name) { - if (virNetDevIPAddrGet(dev_name, &addr) < 0) - goto cleanup; - addrptr = &addr; - } - - if (!(addrptr && - (*netaddr = virSocketAddrFormat(addrptr)))) { - goto cleanup; - } - - ret = 0; - cleanup: - virNetworkObjEndAPI(&obj); - return ret; -} - /* networkGetActualType: * @dom: domain definition that @iface belongs to diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 098f9e5c5b..6fa6432d13 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -36,11 +36,6 @@ networkRegister(void); # if WITH_NETWORK -int -networkGetNetworkAddress(const char *netname, - char **netaddr) - ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); - int networkGetActualType(virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1); @@ -55,7 +50,6 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, # else /* Define no-op replacements that don't drag in any link dependencies. */ # define networkGetActualType(iface) (iface->type) -# define networkGetNetworkAddress(netname, netaddr) (-2) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 7652eadbec..0661e76609 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -60,7 +60,6 @@ #include "domain_audit.h" #include "domain_nwfilter.h" #include "locking/domain_lock.h" -#include "network/bridge_driver.h" #include "viruuid.h" #include "virprocess.h" #include "virtime.h" @@ -4312,9 +4311,95 @@ qemuProcessGraphicsAllocatePorts(virQEMUDriverPtr driver, return 0; } +static int +qemuProcessGetNetworkAddress(virConnectPtr conn, + const char *netname, + char **netaddr) +{ + int ret = -1; + virNetworkPtr net; + virNetworkDefPtr netdef = NULL; + virNetworkIPDefPtr ipdef; + virSocketAddr addr; + virSocketAddrPtr addrptr = NULL; + char *dev_name = NULL; + char *xml = NULL; + + *netaddr = NULL; + net = virNetworkLookupByName(conn, netname); + if (!net) + goto cleanup; + + xml = virNetworkGetXMLDesc(net, 0); + if (!xml) + goto cleanup; + + netdef = virNetworkDefParseString(xml); + if (!netdef) + goto cleanup; + + switch (netdef->forward.type) { + case VIR_NETWORK_FORWARD_NONE: + case VIR_NETWORK_FORWARD_NAT: + case VIR_NETWORK_FORWARD_ROUTE: + case VIR_NETWORK_FORWARD_OPEN: + ipdef = virNetworkDefGetIPByIndex(netdef, AF_UNSPEC, 0); + if (!ipdef) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' doesn't have an IP address"), + netdef->name); + goto cleanup; + } + addrptr = &ipdef->address; + break; + + case VIR_NETWORK_FORWARD_BRIDGE: + if ((dev_name = netdef->bridge)) + break; + /* + * fall through if netdef->bridge wasn't set, since that is + * macvtap bridge mode network. + */ + ATTRIBUTE_FALLTHROUGH; + + case VIR_NETWORK_FORWARD_PRIVATE: + case VIR_NETWORK_FORWARD_VEPA: + case VIR_NETWORK_FORWARD_PASSTHROUGH: + if ((netdef->forward.nifs > 0) && netdef->forward.ifs) + dev_name = netdef->forward.ifs[0].device.dev; + + if (!dev_name) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("network '%s' has no associated interface or bridge"), + netdef->name); + goto cleanup; + } + break; + } + + if (dev_name) { + if (virNetDevIPAddrGet(dev_name, &addr) < 0) + goto cleanup; + addrptr = &addr; + } + + if (!(addrptr && + (*netaddr = virSocketAddrFormat(addrptr)))) { + goto cleanup; + } + + ret = 0; + cleanup: + virNetworkDefFree(netdef); + virObjectUnref(net); + VIR_FREE(xml); + return ret; +} + static int -qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, +qemuProcessGraphicsSetupNetworkAddress(virConnectPtr conn, + virDomainGraphicsListenDefPtr glisten, const char *listenAddr) { int rc; @@ -4326,7 +4411,7 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, return 0; } - rc = networkGetNetworkAddress(glisten->network, &glisten->address); + rc = qemuProcessGetNetworkAddress(conn, glisten->network, &glisten->address); if (rc <= -2) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", _("network-based listen isn't possible, " @@ -4341,7 +4426,8 @@ qemuProcessGraphicsSetupNetworkAddress(virDomainGraphicsListenDefPtr glisten, static int -qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver, +qemuProcessGraphicsSetupListen(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainGraphicsDefPtr graphics, virDomainObjPtr vm) { @@ -4399,7 +4485,8 @@ qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver, if (glisten->address || !listenAddr) continue; - if (qemuProcessGraphicsSetupNetworkAddress(glisten, + if (qemuProcessGraphicsSetupNetworkAddress(conn, + glisten, listenAddr) < 0) goto cleanup; break; @@ -4428,7 +4515,8 @@ qemuProcessGraphicsSetupListen(virQEMUDriverPtr driver, static int -qemuProcessSetupGraphics(virQEMUDriverPtr driver, +qemuProcessSetupGraphics(virConnectPtr conn, + virQEMUDriverPtr driver, virDomainObjPtr vm, unsigned int flags) { @@ -4440,7 +4528,7 @@ qemuProcessSetupGraphics(virQEMUDriverPtr driver, for (i = 0; i < vm->def->ngraphics; i++) { graphics = vm->def->graphics[i]; - if (qemuProcessGraphicsSetupListen(driver, graphics, vm) < 0) + if (qemuProcessGraphicsSetupListen(conn, driver, graphics, vm) < 0) goto cleanup; } @@ -5596,7 +5684,7 @@ qemuProcessPrepareDomain(virConnectPtr conn, goto cleanup; VIR_DEBUG("Setting graphics devices"); - if (qemuProcessSetupGraphics(driver, vm, flags) < 0) + if (qemuProcessSetupGraphics(conn, driver, vm, flags) < 0) goto cleanup; VIR_DEBUG("Create domain masterKey"); -- 2.14.3
Currently the QEMU driver will call directly into the network driver impl to modify resolve the atual type of NICs with type=network. It has todo this before it has allocated the actual NIC. This introduces a callback system to allow us to decouple the QEMU driver from the network driver. This is a short term step, as it ought to be possible to achieve the same end goal by simply querying XML via the public network API. The QEMU code in question though, has no virConnectPtr conveniently available at this time. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++++- src/conf/domain_conf.h | 15 ++++++++++++++- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 10 +++++----- src/network/bridge_driver.h | 5 ----- src/qemu/qemu_alias.c | 3 +-- src/qemu/qemu_domain_address.c | 3 +-- tests/Makefile.am | 11 +++++++++-- tests/qemuxml2argvtest.c | 4 ++++ 9 files changed, 51 insertions(+), 18 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 205f99618d..80d349fc5a 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28822,6 +28822,7 @@ static virDomainNetNotifyActualDeviceImpl netNotify; static virDomainNetReleaseActualDeviceImpl netRelease; static virDomainNetBandwidthChangeAllowedImpl netBandwidthChangeAllowed; static virDomainNetBandwidthUpdateImpl netBandwidthUpdate; +static virDomainNetResolveActualTypeImpl netResolveActualType; void @@ -28829,13 +28830,15 @@ virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, virDomainNetReleaseActualDeviceImpl release, virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, - virDomainNetBandwidthUpdateImpl bandwidthUpdate) + virDomainNetBandwidthUpdateImpl bandwidthUpdate, + virDomainNetResolveActualTypeImpl resolveActualType) { netAllocate = allocate; netNotify = notify; netRelease = release; netBandwidthChangeAllowed = bandwidthChangeAllowed; netBandwidthUpdate = bandwidthUpdate; + netResolveActualType = resolveActualType; } int @@ -28903,3 +28906,15 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, return netBandwidthUpdate(iface, newBandwidth); } + +int +virDomainNetResolveActualType(virDomainNetDefPtr iface) +{ + if (!netResolveActualType) { + virReportError(VIR_ERR_NO_SUPPORT, "%s", + _("Network device resolve type not available")); + return -1; + } + + return netResolveActualType(iface); +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 32948e181c..7a2f434ec8 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3474,13 +3474,17 @@ typedef int (*virDomainNetBandwidthUpdateImpl)(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth); +typedef int +(*virDomainNetResolveActualTypeImpl)(virDomainNetDefPtr iface); + void virDomainNetSetDeviceImpl(virDomainNetAllocateActualDeviceImpl allocate, virDomainNetNotifyActualDeviceImpl notify, virDomainNetReleaseActualDeviceImpl release, virDomainNetBandwidthChangeAllowedImpl bandwidthChangeAllowed, - virDomainNetBandwidthUpdateImpl bandwidthUpdate); + virDomainNetBandwidthUpdateImpl bandwidthUpdate, + virDomainNetResolveActualTypeImpl resolveActualType); int virDomainNetAllocateActualDevice(virDomainDefPtr dom, @@ -3507,5 +3511,14 @@ virDomainNetBandwidthUpdate(virDomainNetDefPtr iface, virNetDevBandwidthPtr newBandwidth) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2); +/* XXX this is a nasty hack and should be removed. It should + * be by via public API by fetching XML and parsing it. Not + * easy right now as code paths in QEMU reying on this don't + * have a virConnectPtr handy. + */ +int +virDomainNetResolveActualType(virDomainNetDefPtr iface) + ATTRIBUTE_NONNULL(1); + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 45ae25e57c..f2801a0d73 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -459,6 +459,7 @@ virDomainNetNotifyActualDevice; virDomainNetReleaseActualDevice; virDomainNetRemove; virDomainNetRemoveHostdev; +virDomainNetResolveActualType; virDomainNetSetDeviceImpl; virDomainNetTypeFromString; virDomainNetTypeSharesHostView; diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 9c0b3c0895..dd6e3402ea 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -5147,8 +5147,7 @@ networkReleaseActualDevice(virDomainDefPtr dom, -/* networkGetActualType: - * @dom: domain definition that @iface belongs to +/* networkResolveActualType: * @iface: the original NetDef from the domain * * Looks up the network reference by iface, and returns the actual @@ -5156,8 +5155,8 @@ networkReleaseActualDevice(virDomainDefPtr dom, * * Returns 0 on success, -1 on failure. */ -int -networkGetActualType(virDomainNetDefPtr iface) +static int +networkResolveActualType(virDomainNetDefPtr iface) { virNetworkDriverStatePtr driver = networkGetDriver(); virNetworkObjPtr obj = NULL; @@ -5718,7 +5717,8 @@ networkRegister(void) networkNotifyActualDevice, networkReleaseActualDevice, networkBandwidthChangeAllowed, - networkBandwidthUpdate); + networkBandwidthUpdate, + networkResolveActualType); return 0; } diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 6fa6432d13..2cf886a7e6 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -36,10 +36,6 @@ networkRegister(void); # if WITH_NETWORK -int -networkGetActualType(virDomainNetDefPtr iface) - ATTRIBUTE_NONNULL(1); - int networkDnsmasqConfContents(virNetworkObjPtr obj, const char *pidfile, @@ -49,7 +45,6 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, # else /* Define no-op replacements that don't drag in any link dependencies. */ -# define networkGetActualType(iface) (iface->type) # define networkDnsmasqConfContents(network, pidfile, configstr, \ dctx, caps) 0 diff --git a/src/qemu/qemu_alias.c b/src/qemu/qemu_alias.c index e8c1256b75..ae30f7df82 100644 --- a/src/qemu/qemu_alias.c +++ b/src/qemu/qemu_alias.c @@ -27,7 +27,6 @@ #include "viralloc.h" #include "virlog.h" #include "virstring.h" -#include "network/bridge_driver.h" #define QEMU_DRIVE_HOST_PREFIX "drive-" @@ -271,7 +270,7 @@ qemuAssignDeviceNetAlias(virDomainDefPtr def, * We must use "-1" as the index because the caller doesn't know * that we're now looking for a unique hostdevN rather than netN */ - if (networkGetActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) + if (virDomainNetResolveActualType(net) == VIR_DOMAIN_NET_TYPE_HOSTDEV) return qemuAssignDeviceHostdevAlias(def, &net->info.alias, -1); if (idx == -1) { diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 6272b1e91d..e28119e4b1 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -25,7 +25,6 @@ #include "qemu_domain_address.h" #include "qemu_domain.h" -#include "network/bridge_driver.h" #include "viralloc.h" #include "virerror.h" #include "virlog.h" @@ -1064,7 +1063,7 @@ qemuDomainFillDeviceIsolationGroup(virDomainDefPtr def, * to is of type hostdev. All other kinds of network interfaces don't * require us to isolate the guest device, so we can skip them */ if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK || - networkGetActualType(iface) != VIR_DOMAIN_NET_TYPE_HOSTDEV) { + virDomainNetResolveActualType(iface) != VIR_DOMAIN_NET_TYPE_HOSTDEV) { goto skip; } diff --git a/tests/Makefile.am b/tests/Makefile.am index 497bd21a25..d013aed5eb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -280,7 +280,7 @@ test_libraries += virmocklibxl.la endif WITH_LIBXL if WITH_QEMU -test_programs += qemuxml2argvtest qemuxml2xmltest \ +test_programs += qemuxml2xmltest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ qemuagenttest qemucapabilitiestest qemucaps2xmltest \ @@ -288,6 +288,11 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemucommandutiltest \ qemublocktest \ $(NULL) +if WITH_NETWORK +# Dep on the network driver callback for resolving NIC +# actual type. XXX remove this dep. +test_programs += qemuxml2xmltest +endif WITH_NETWORK test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ libqemutestdriver.la \ @@ -582,7 +587,9 @@ qemucpumock_la_LIBADD = $(MOCKLIBS_LIBS) qemuxml2argvtest_SOURCES = \ qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2argvtest_LDADD = libqemutestdriver.la $(LDADDS) $(LIBXML_LIBS) +qemuxml2argvtest_LDADD = libqemutestdriver.la \ + ../src/libvirt_driver_network_impl.la \ + $(LDADDS) $(LIBXML_LIBS) qemuxml2argvmock_la_SOURCES = \ qemuxml2argvmock.c diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index c8739909de..b341467143 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -25,6 +25,7 @@ # include "cpu/cpu_map.h" # include "virstring.h" # include "storage/storage_driver.h" +# include "network/bridge_driver.h" # include "virmock.h" # define __QEMU_CAPSPRIV_H_ALLOW__ @@ -580,6 +581,9 @@ mymain(void) if (qemuTestDriverInit(&driver) < 0) return EXIT_FAILURE; + if (networkRegister() < 0) + return EXIT_FAILURE; + driver.privileged = true; VIR_FREE(driver.config->defaultTLSx509certdir); -- 2.14.3
On 02/05/2018 10:28 AM, Daniel P. Berrangé wrote:
Currently the QEMU driver will call directly into the network driver impl to modify resolve the atual type of NICs with type=network. It has todo this before it has allocated the actual NIC. This introduces a callback system to allow us to decouple the QEMU driver from the network driver.
This is a short term step, as it ought to be possible to achieve the same end goal by simply querying XML via the public network API. The QEMU code in question though, has no virConnectPtr conveniently available at this time.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++++- src/conf/domain_conf.h | 15 ++++++++++++++- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 10 +++++----- src/network/bridge_driver.h | 5 ----- src/qemu/qemu_alias.c | 3 +-- src/qemu/qemu_domain_address.c | 3 +-- tests/Makefile.am | 11 +++++++++-- tests/qemuxml2argvtest.c | 4 ++++ 9 files changed, 51 insertions(+), 18 deletions(-)
I realized while reviewing Andrea's changes just now that the xml2argvtest is missing! and I think I know why... [...]
diff --git a/tests/Makefile.am b/tests/Makefile.am index 497bd21a25..d013aed5eb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -280,7 +280,7 @@ test_libraries += virmocklibxl.la endif WITH_LIBXL
if WITH_QEMU -test_programs += qemuxml2argvtest qemuxml2xmltest \ ^^^^^^^^^^^^^^^^ hmmm...
+test_programs += qemuxml2xmltest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ qemuagenttest qemucapabilitiestest qemucaps2xmltest \ @@ -288,6 +288,11 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemucommandutiltest \ qemublocktest \ $(NULL) +if WITH_NETWORK +# Dep on the network driver callback for resolving NIC +# actual type. XXX remove this dep. +test_programs += qemuxml2xmltest
Was this supposed to be += qemuxml2argvtest ?? John
+endif WITH_NETWORK test_helpers += qemucapsprobe test_libraries += libqemumonitortestutils.la \ libqemutestdriver.la \ @@ -582,7 +587,9 @@ qemucpumock_la_LIBADD = $(MOCKLIBS_LIBS) qemuxml2argvtest_SOURCES = \ qemuxml2argvtest.c testutilsqemu.c testutilsqemu.h \ testutils.c testutils.h -qemuxml2argvtest_LDADD = libqemutestdriver.la $(LDADDS) $(LIBXML_LIBS) +qemuxml2argvtest_LDADD = libqemutestdriver.la \ + ../src/libvirt_driver_network_impl.la \ + $(LDADDS) $(LIBXML_LIBS)
qemuxml2argvmock_la_SOURCES = \ qemuxml2argvmock.c
On Fri, Feb 09, 2018 at 05:09:36PM -0500, John Ferlan wrote:
On 02/05/2018 10:28 AM, Daniel P. Berrangé wrote:
Currently the QEMU driver will call directly into the network driver impl to modify resolve the atual type of NICs with type=network. It has todo this before it has allocated the actual NIC. This introduces a callback system to allow us to decouple the QEMU driver from the network driver.
This is a short term step, as it ought to be possible to achieve the same end goal by simply querying XML via the public network API. The QEMU code in question though, has no virConnectPtr conveniently available at this time.
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/conf/domain_conf.c | 17 ++++++++++++++++- src/conf/domain_conf.h | 15 ++++++++++++++- src/libvirt_private.syms | 1 + src/network/bridge_driver.c | 10 +++++----- src/network/bridge_driver.h | 5 ----- src/qemu/qemu_alias.c | 3 +-- src/qemu/qemu_domain_address.c | 3 +-- tests/Makefile.am | 11 +++++++++-- tests/qemuxml2argvtest.c | 4 ++++ 9 files changed, 51 insertions(+), 18 deletions(-)
I realized while reviewing Andrea's changes just now that the xml2argvtest is missing! and I think I know why...
[...]
diff --git a/tests/Makefile.am b/tests/Makefile.am index 497bd21a25..d013aed5eb 100644 --- a/tests/Makefile.am +++ b/tests/Makefile.am @@ -280,7 +280,7 @@ test_libraries += virmocklibxl.la endif WITH_LIBXL
if WITH_QEMU -test_programs += qemuxml2argvtest qemuxml2xmltest \ ^^^^^^^^^^^^^^^^ hmmm...
+test_programs += qemuxml2xmltest \ qemuargv2xmltest qemuhelptest domainsnapshotxml2xmltest \ qemumonitortest qemumonitorjsontest qemuhotplugtest \ qemuagenttest qemucapabilitiestest qemucaps2xmltest \ @@ -288,6 +288,11 @@ test_programs += qemuxml2argvtest qemuxml2xmltest \ qemucommandutiltest \ qemublocktest \ $(NULL) +if WITH_NETWORK +# Dep on the network driver callback for resolving NIC +# actual type. XXX remove this dep. +test_programs += qemuxml2xmltest
Was this supposed to be += qemuxml2argvtest ??
Sigh, rebase / merge error :-( 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 :|
The networkDnsmasqConfContents() method is only used by the test suite and that's only built with WITH_NETWORK is set. So there is no longer any reason to conditionalize the declaration of this method. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/network/bridge_driver.h | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/network/bridge_driver.h b/src/network/bridge_driver.h index 2cf886a7e6..b70881a690 100644 --- a/src/network/bridge_driver.h +++ b/src/network/bridge_driver.h @@ -34,8 +34,6 @@ int networkRegister(void); -# if WITH_NETWORK - int networkDnsmasqConfContents(virNetworkObjPtr obj, const char *pidfile, @@ -43,11 +41,4 @@ networkDnsmasqConfContents(virNetworkObjPtr obj, dnsmasqContext *dctx, dnsmasqCapsPtr caps); -# else -/* Define no-op replacements that don't drag in any link dependencies. */ -# define networkDnsmasqConfContents(network, pidfile, configstr, \ - dctx, caps) 0 - -# endif - #endif /* __VIR_NETWORK__DRIVER_H */ -- 2.14.3
The virStorageTranslateDiskSourcePool method modifies a virDomainDiskDef to resolve any storage pool reference. For some reason this was added into the storage driver code, despite working entirely in terms of the public APIs. Move it into the domain conf file and rename it to match the object it modifies. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/bhyve/bhyve_command.c | 7 +- src/conf/domain_conf.c | 253 +++++++++++++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 4 + src/libvirt_private.syms | 1 + src/qemu/qemu_driver.c | 9 +- src/qemu/qemu_hotplug.c | 3 +- src/qemu/qemu_process.c | 5 +- src/storage/storage_driver.c | 251 ------------------------------------------ src/storage/storage_driver.h | 3 - 9 files changed, 268 insertions(+), 268 deletions(-) diff --git a/src/bhyve/bhyve_command.c b/src/bhyve/bhyve_command.c index 55032ae1df..c1241b8110 100644 --- a/src/bhyve/bhyve_command.c +++ b/src/bhyve/bhyve_command.c @@ -37,7 +37,6 @@ #include "virnetdev.h" #include "virnetdevbridge.h" #include "virnetdevtap.h" -#include "storage/storage_driver.h" #define VIR_FROM_THIS VIR_FROM_BHYVE @@ -199,7 +198,7 @@ bhyveBuildAHCIControllerArgStr(const virDomainDef *def, goto error; } - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) goto error; disk_source = virDomainDiskGetSource(disk); @@ -295,7 +294,7 @@ bhyveBuildVirtIODiskArgStr(const virDomainDef *def ATTRIBUTE_UNUSED, { const char *disk_source; - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) return -1; if (disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) { @@ -676,7 +675,7 @@ static bool virBhyveUsableDisk(virConnectPtr conn, virDomainDiskDefPtr disk) { - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) return false; if ((disk->device != VIR_DOMAIN_DISK_DEVICE_DISK) && diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 80d349fc5a..468d07ba11 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -28918,3 +28918,256 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface) return netResolveActualType(iface); } + + +static int +virDomainDiskAddISCSIPoolSourceHost(virDomainDiskDefPtr def, + virStoragePoolDefPtr pooldef) +{ + int ret = -1; + char **tokens = NULL; + + /* Only support one host */ + if (pooldef->source.nhost != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Expected exactly 1 host for the storage pool")); + goto cleanup; + } + + /* iscsi pool only supports one host */ + def->src->nhosts = 1; + + if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0) + goto cleanup; + + if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0) + goto cleanup; + + def->src->hosts[0].port = pooldef->source.hosts[0].port ? + pooldef->source.hosts[0].port : 3260; + + /* iscsi volume has name like "unit:0:0:1" */ + if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0))) + goto cleanup; + + if (virStringListLength((const char * const *)tokens) != 4) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected iscsi volume name '%s'"), + def->src->srcpool->volume); + goto cleanup; + } + + /* iscsi pool has only one source device path */ + if (virAsprintf(&def->src->path, "%s/%s", + pooldef->source.devices[0].path, + tokens[3]) < 0) + goto cleanup; + + /* Storage pool have not supported these 2 attributes yet, + * use the defaults. + */ + def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; + def->src->hosts[0].socket = NULL; + + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + + ret = 0; + + cleanup: + virStringListFree(tokens); + return ret; +} + + +static int +virDomainDiskTranslateSourcePoolAuth(virDomainDiskDefPtr def, + virStoragePoolSourcePtr source) +{ + int ret = -1; + + /* Only necessary when authentication set */ + if (!source->auth) { + ret = 0; + goto cleanup; + } + def->src->auth = virStorageAuthDefCopy(source->auth); + if (!def->src->auth) + goto cleanup; + /* A <disk> doesn't use <auth type='%s', so clear that out for the disk */ + def->src->auth->authType = VIR_STORAGE_AUTH_TYPE_NONE; + ret = 0; + + cleanup: + return ret; +} + + +int +virDomainDiskTranslateSourcePool(virConnectPtr conn, + virDomainDiskDefPtr def) +{ + virStoragePoolDefPtr pooldef = NULL; + virStoragePoolPtr pool = NULL; + virStorageVolPtr vol = NULL; + char *poolxml = NULL; + virStorageVolInfo info; + int ret = -1; + + if (def->src->type != VIR_STORAGE_TYPE_VOLUME) + return 0; + + if (!def->src->srcpool) + return 0; + + if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool))) + return -1; + + if (virStoragePoolIsActive(pool) != 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("storage pool '%s' containing volume '%s' " + "is not active"), + def->src->srcpool->pool, def->src->srcpool->volume); + goto cleanup; + } + + if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume))) + goto cleanup; + + if (virStorageVolGetInfo(vol, &info) < 0) + goto cleanup; + + if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) + goto cleanup; + + if (!(pooldef = virStoragePoolDefParseString(poolxml))) + goto cleanup; + + def->src->srcpool->pooltype = pooldef->type; + def->src->srcpool->voltype = info.type; + + if (def->src->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("disk source mode is only valid when " + "storage pool is of iscsi type")); + goto cleanup; + } + + VIR_FREE(def->src->path); + virStorageNetHostDefFree(def->src->nhosts, def->src->hosts); + def->src->nhosts = 0; + def->src->hosts = NULL; + virStorageAuthDefFree(def->src->auth); + def->src->auth = NULL; + + switch ((virStoragePoolType) pooldef->type) { + case VIR_STORAGE_POOL_DIR: + case VIR_STORAGE_POOL_FS: + case VIR_STORAGE_POOL_NETFS: + case VIR_STORAGE_POOL_LOGICAL: + case VIR_STORAGE_POOL_DISK: + case VIR_STORAGE_POOL_SCSI: + case VIR_STORAGE_POOL_ZFS: + case VIR_STORAGE_POOL_VSTORAGE: + if (!(def->src->path = virStorageVolGetPath(vol))) + goto cleanup; + + if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + + switch (info.type) { + case VIR_STORAGE_VOL_FILE: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; + break; + + case VIR_STORAGE_VOL_DIR: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR; + break; + + case VIR_STORAGE_VOL_BLOCK: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; + break; + + case VIR_STORAGE_VOL_PLOOP: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; + break; + + case VIR_STORAGE_VOL_NETWORK: + case VIR_STORAGE_VOL_NETDIR: + virReportError(VIR_ERR_INTERNAL_ERROR, + _("unexpected storage volume type '%s' " + "for storage pool type '%s'"), + virStorageVolTypeToString(info.type), + virStoragePoolTypeToString(pooldef->type)); + goto cleanup; + } + + break; + + case VIR_STORAGE_POOL_ISCSI: + if (def->startupPolicy) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("'startupPolicy' is only valid for " + "'file' type volume")); + goto cleanup; + } + + switch (def->src->srcpool->mode) { + case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT: + case VIR_STORAGE_SOURCE_POOL_MODE_LAST: + def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST; + ATTRIBUTE_FALLTHROUGH; + case VIR_STORAGE_SOURCE_POOL_MODE_HOST: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; + if (!(def->src->path = virStorageVolGetPath(vol))) + goto cleanup; + break; + + case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: + def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; + def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; + + if (virDomainDiskTranslateSourcePoolAuth(def, + &pooldef->source) < 0) + goto cleanup; + + /* Source pool may not fill in the secrettype field, + * so we need to do so here + */ + if (def->src->auth && !def->src->auth->secrettype) { + const char *secrettype = + virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); + if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0) + goto cleanup; + } + + if (virDomainDiskAddISCSIPoolSourceHost(def, pooldef) < 0) + goto cleanup; + break; + } + break; + + case VIR_STORAGE_POOL_MPATH: + case VIR_STORAGE_POOL_RBD: + case VIR_STORAGE_POOL_SHEEPDOG: + case VIR_STORAGE_POOL_GLUSTER: + case VIR_STORAGE_POOL_LAST: + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, + _("using '%s' pools for backing 'volume' disks " + "isn't yet supported"), + virStoragePoolTypeToString(pooldef->type)); + goto cleanup; + } + + ret = 0; + cleanup: + virObjectUnref(pool); + virObjectUnref(vol); + VIR_FREE(poolxml); + virStoragePoolDefFree(pooldef); + return ret; +} diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7a2f434ec8..63f56a4557 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -3521,4 +3521,8 @@ virDomainNetResolveActualType(virDomainNetDefPtr iface) ATTRIBUTE_NONNULL(1); +int virDomainDiskTranslateSourcePool(virConnectPtr conn, + virDomainDiskDefPtr def); + + #endif /* __DOMAIN_CONF_H */ diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index f2801a0d73..3b14d7d158 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -345,6 +345,7 @@ virDomainDiskSetDriver; virDomainDiskSetFormat; virDomainDiskSetSource; virDomainDiskSetType; +virDomainDiskTranslateSourcePool; virDomainFSDefFree; virDomainFSDefNew; virDomainFSDriverTypeToString; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index c9bbe0ca37..dfad894999 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -99,7 +99,6 @@ #include "virstring.h" #include "viraccessapicheck.h" #include "viraccessapicheckqemu.h" -#include "storage/storage_driver.h" #include "virhostdev.h" #include "domain_capabilities.h" #include "vircgroup.h" @@ -7838,7 +7837,7 @@ qemuDomainChangeDiskLive(virConnectPtr conn, virDomainDiskDefPtr orig_disk = NULL; int ret = -1; - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) goto cleanup; if (qemuDomainDetermineDiskChain(driver, vm, disk, false, true) < 0) @@ -7965,7 +7964,7 @@ qemuDomainAttachDeviceConfig(virDomainDefPtr vmdef, _("target %s already exists"), disk->dst); return -1; } - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) return -1; if (qemuCheckDiskConfig(disk, NULL) < 0) return -1; @@ -14121,7 +14120,7 @@ qemuDomainSnapshotPrepareDiskExternal(virConnectPtr conn, return -1; if (!active) { - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) return -1; if (qemuDomainSnapshotPrepareDiskExternalInactive(snapdisk, disk) < 0) @@ -14173,7 +14172,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, if (active) return 0; - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) return -1; actualType = virStorageSourceGetActualType(disk->src); diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 1a406b6d26..c7bf25eeef 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -56,7 +56,6 @@ #include "virstoragefile.h" #include "virstring.h" #include "virtime.h" -#include "storage/storage_driver.h" #define VIR_FROM_THIS VIR_FROM_QEMU @@ -714,7 +713,7 @@ qemuDomainAttachDeviceDiskLive(virConnectPtr conn, goto cleanup; } - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) goto cleanup; if (qemuAddSharedDevice(driver, dev, vm->def->name) < 0) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 0661e76609..2588c2ca11 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -72,7 +72,6 @@ #include "virstring.h" #include "virhostdev.h" #include "secret_util.h" -#include "storage/storage_driver.h" #include "configmake.h" #include "nwfilter_conf.h" #include "netdev_bandwidth_conf.h" @@ -5573,7 +5572,7 @@ qemuProcessPrepareDomainStorage(virConnectPtr conn, size_t idx = i - 1; virDomainDiskDefPtr disk = vm->def->disks[idx]; - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) { + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) { if (qemuDomainCheckDiskStartupPolicy(driver, vm, idx, cold_boot) < 0) return -1; @@ -7333,7 +7332,7 @@ qemuProcessReconnect(void *opaque) virDomainDiskDefPtr disk = obj->def->disks[i]; virDomainDeviceDef dev; - if (virStorageTranslateDiskSourcePool(conn, disk) < 0) + if (virDomainDiskTranslateSourcePool(conn, disk) < 0) goto error; /* backing chains need to be refreshed only if they could change */ diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index f2afb0b3f9..1b4bce4fc8 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -2752,257 +2752,6 @@ storageConnectStoragePoolEventDeregisterAny(virConnectPtr conn, } -static int -virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def, - virStoragePoolDefPtr pooldef) -{ - int ret = -1; - char **tokens = NULL; - - /* Only support one host */ - if (pooldef->source.nhost != 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("Expected exactly 1 host for the storage pool")); - goto cleanup; - } - - /* iscsi pool only supports one host */ - def->src->nhosts = 1; - - if (VIR_ALLOC_N(def->src->hosts, def->src->nhosts) < 0) - goto cleanup; - - if (VIR_STRDUP(def->src->hosts[0].name, pooldef->source.hosts[0].name) < 0) - goto cleanup; - - def->src->hosts[0].port = pooldef->source.hosts[0].port ? - pooldef->source.hosts[0].port : 3260; - - /* iscsi volume has name like "unit:0:0:1" */ - if (!(tokens = virStringSplit(def->src->srcpool->volume, ":", 0))) - goto cleanup; - - if (virStringListLength((const char * const *)tokens) != 4) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected iscsi volume name '%s'"), - def->src->srcpool->volume); - goto cleanup; - } - - /* iscsi pool has only one source device path */ - if (virAsprintf(&def->src->path, "%s/%s", - pooldef->source.devices[0].path, - tokens[3]) < 0) - goto cleanup; - - /* Storage pool have not supported these 2 attributes yet, - * use the defaults. - */ - def->src->hosts[0].transport = VIR_STORAGE_NET_HOST_TRANS_TCP; - def->src->hosts[0].socket = NULL; - - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - - ret = 0; - - cleanup: - virStringListFree(tokens); - return ret; -} - - -static int -virStorageTranslateDiskSourcePoolAuth(virDomainDiskDefPtr def, - virStoragePoolSourcePtr source) -{ - int ret = -1; - - /* Only necessary when authentication set */ - if (!source->auth) { - ret = 0; - goto cleanup; - } - def->src->auth = virStorageAuthDefCopy(source->auth); - if (!def->src->auth) - goto cleanup; - /* A <disk> doesn't use <auth type='%s', so clear that out for the disk */ - def->src->auth->authType = VIR_STORAGE_AUTH_TYPE_NONE; - ret = 0; - - cleanup: - return ret; -} - - -int -virStorageTranslateDiskSourcePool(virConnectPtr conn, - virDomainDiskDefPtr def) -{ - virStoragePoolDefPtr pooldef = NULL; - virStoragePoolPtr pool = NULL; - virStorageVolPtr vol = NULL; - char *poolxml = NULL; - virStorageVolInfo info; - int ret = -1; - - if (def->src->type != VIR_STORAGE_TYPE_VOLUME) - return 0; - - if (!def->src->srcpool) - return 0; - - if (!(pool = virStoragePoolLookupByName(conn, def->src->srcpool->pool))) - return -1; - - if (virStoragePoolIsActive(pool) != 1) { - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("storage pool '%s' containing volume '%s' " - "is not active"), - def->src->srcpool->pool, def->src->srcpool->volume); - goto cleanup; - } - - if (!(vol = virStorageVolLookupByName(pool, def->src->srcpool->volume))) - goto cleanup; - - if (virStorageVolGetInfo(vol, &info) < 0) - goto cleanup; - - if (!(poolxml = virStoragePoolGetXMLDesc(pool, 0))) - goto cleanup; - - if (!(pooldef = virStoragePoolDefParseString(poolxml))) - goto cleanup; - - def->src->srcpool->pooltype = pooldef->type; - def->src->srcpool->voltype = info.type; - - if (def->src->srcpool->mode && pooldef->type != VIR_STORAGE_POOL_ISCSI) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("disk source mode is only valid when " - "storage pool is of iscsi type")); - goto cleanup; - } - - VIR_FREE(def->src->path); - virStorageNetHostDefFree(def->src->nhosts, def->src->hosts); - def->src->nhosts = 0; - def->src->hosts = NULL; - virStorageAuthDefFree(def->src->auth); - def->src->auth = NULL; - - switch ((virStoragePoolType) pooldef->type) { - case VIR_STORAGE_POOL_DIR: - case VIR_STORAGE_POOL_FS: - case VIR_STORAGE_POOL_NETFS: - case VIR_STORAGE_POOL_LOGICAL: - case VIR_STORAGE_POOL_DISK: - case VIR_STORAGE_POOL_SCSI: - case VIR_STORAGE_POOL_ZFS: - case VIR_STORAGE_POOL_VSTORAGE: - if (!(def->src->path = virStorageVolGetPath(vol))) - goto cleanup; - - if (def->startupPolicy && info.type != VIR_STORAGE_VOL_FILE) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - goto cleanup; - } - - - switch (info.type) { - case VIR_STORAGE_VOL_FILE: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; - break; - - case VIR_STORAGE_VOL_DIR: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_DIR; - break; - - case VIR_STORAGE_VOL_BLOCK: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; - break; - - case VIR_STORAGE_VOL_PLOOP: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_FILE; - break; - - case VIR_STORAGE_VOL_NETWORK: - case VIR_STORAGE_VOL_NETDIR: - virReportError(VIR_ERR_INTERNAL_ERROR, - _("unexpected storage volume type '%s' " - "for storage pool type '%s'"), - virStorageVolTypeToString(info.type), - virStoragePoolTypeToString(pooldef->type)); - goto cleanup; - } - - break; - - case VIR_STORAGE_POOL_ISCSI: - if (def->startupPolicy) { - virReportError(VIR_ERR_XML_ERROR, "%s", - _("'startupPolicy' is only valid for " - "'file' type volume")); - goto cleanup; - } - - switch (def->src->srcpool->mode) { - case VIR_STORAGE_SOURCE_POOL_MODE_DEFAULT: - case VIR_STORAGE_SOURCE_POOL_MODE_LAST: - def->src->srcpool->mode = VIR_STORAGE_SOURCE_POOL_MODE_HOST; - ATTRIBUTE_FALLTHROUGH; - case VIR_STORAGE_SOURCE_POOL_MODE_HOST: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_BLOCK; - if (!(def->src->path = virStorageVolGetPath(vol))) - goto cleanup; - break; - - case VIR_STORAGE_SOURCE_POOL_MODE_DIRECT: - def->src->srcpool->actualtype = VIR_STORAGE_TYPE_NETWORK; - def->src->protocol = VIR_STORAGE_NET_PROTOCOL_ISCSI; - - if (virStorageTranslateDiskSourcePoolAuth(def, - &pooldef->source) < 0) - goto cleanup; - - /* Source pool may not fill in the secrettype field, - * so we need to do so here - */ - if (def->src->auth && !def->src->auth->secrettype) { - const char *secrettype = - virSecretUsageTypeToString(VIR_SECRET_USAGE_TYPE_ISCSI); - if (VIR_STRDUP(def->src->auth->secrettype, secrettype) < 0) - goto cleanup; - } - - if (virStorageAddISCSIPoolSourceHost(def, pooldef) < 0) - goto cleanup; - break; - } - break; - - case VIR_STORAGE_POOL_MPATH: - case VIR_STORAGE_POOL_RBD: - case VIR_STORAGE_POOL_SHEEPDOG: - case VIR_STORAGE_POOL_GLUSTER: - case VIR_STORAGE_POOL_LAST: - virReportError(VIR_ERR_CONFIG_UNSUPPORTED, - _("using '%s' pools for backing 'volume' disks " - "isn't yet supported"), - virStoragePoolTypeToString(pooldef->type)); - goto cleanup; - } - - ret = 0; - cleanup: - virObjectUnref(pool); - virObjectUnref(vol); - VIR_FREE(poolxml); - virStoragePoolDefFree(pooldef); - return ret; -} /* diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index fd2fae1ba2..c284bf520f 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -29,9 +29,6 @@ # include "domain_conf.h" # include "virstorageobj.h" -int virStorageTranslateDiskSourcePool(virConnectPtr conn, - virDomainDiskDefPtr def); - virStoragePoolObjPtr virStoragePoolObjFindPoolByUUID(const unsigned char *uuid) ATTRIBUTE_NONNULL(1); -- 2.14.3
The storagePoolLookupByTargetPath() method in the storage driver is used by the QEMU driver during block migration. If there's a valid use case for this in the QEMU driver, then external apps likely have similar needs. Exposing it in the public API removes the direct dependancy from the QEMU driver to the storage driver. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- include/libvirt/libvirt-storage.h | 2 ++ src/driver-storage.h | 5 +++++ src/libvirt-storage.c | 40 +++++++++++++++++++++++++++++++++++++++ src/libvirt_public.syms | 6 ++++++ src/qemu/qemu_migration.c | 2 +- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 17 ++++++++++++++++- src/remote_protocol-structs | 7 +++++++ src/storage/storage_driver.c | 5 +++++ 9 files changed, 83 insertions(+), 2 deletions(-) diff --git a/include/libvirt/libvirt-storage.h b/include/libvirt/libvirt-storage.h index 736e2e3b80..413d9f6c4c 100644 --- a/include/libvirt/libvirt-storage.h +++ b/include/libvirt/libvirt-storage.h @@ -264,6 +264,8 @@ virStoragePoolPtr virStoragePoolLookupByUUID (virConnectPtr conn, virStoragePoolPtr virStoragePoolLookupByUUIDString(virConnectPtr conn, const char *uuid); virStoragePoolPtr virStoragePoolLookupByVolume (virStorageVolPtr vol); +virStoragePoolPtr virStoragePoolLookupByTargetPath(virConnectPtr conn, + const char *path); /* * Creating/destroying pools diff --git a/src/driver-storage.h b/src/driver-storage.h index 48e588a546..146eb88b2c 100644 --- a/src/driver-storage.h +++ b/src/driver-storage.h @@ -63,6 +63,10 @@ typedef virStoragePoolPtr typedef virStoragePoolPtr (*virDrvStoragePoolLookupByVolume)(virStorageVolPtr vol); +typedef virStoragePoolPtr +(*virDrvStoragePoolLookupByTargetPath)(virConnectPtr conn, + const char *path); + typedef virStoragePoolPtr (*virDrvStoragePoolCreateXML)(virConnectPtr conn, const char *xmlDesc, @@ -236,6 +240,7 @@ struct _virStorageDriver { virDrvStoragePoolLookupByName storagePoolLookupByName; virDrvStoragePoolLookupByUUID storagePoolLookupByUUID; virDrvStoragePoolLookupByVolume storagePoolLookupByVolume; + virDrvStoragePoolLookupByTargetPath storagePoolLookupByTargetPath; virDrvStoragePoolCreateXML storagePoolCreateXML; virDrvStoragePoolDefineXML storagePoolDefineXML; virDrvStoragePoolBuild storagePoolBuild; diff --git a/src/libvirt-storage.c b/src/libvirt-storage.c index e4646cb80f..3845a5d55e 100644 --- a/src/libvirt-storage.c +++ b/src/libvirt-storage.c @@ -497,6 +497,46 @@ virStoragePoolLookupByVolume(virStorageVolPtr vol) } +/** + * virStoragePoolLookupByTargetPath: + * @conn: pointer to hypervisor connection + * @path: path at which the pool is exposed + * + * Fetch a storage pool which maps to a particular target directory. + * If more than one pool maps to the path, it is undefined which + * will be returned first. + * + * virStoragePoolFree should be used to free the resources after the + * storage pool object is no longer needed. + * + * Returns a virStoragePoolPtr object, or NULL if no matching pool is found + */ +virStoragePoolPtr +virStoragePoolLookupByTargetPath(virConnectPtr conn, + const char *path) +{ + VIR_DEBUG("conn=%p, path=%s", conn, NULLSTR(path)); + + virResetLastError(); + + virCheckConnectReturn(conn, NULL); + virCheckNonNullArgGoto(path, error); + + if (conn->storageDriver && conn->storageDriver->storagePoolLookupByTargetPath) { + virStoragePoolPtr ret; + ret = conn->storageDriver->storagePoolLookupByTargetPath(conn, path); + if (!ret) + goto error; + return ret; + } + + virReportUnsupportedError(); + + error: + virDispatchError(conn); + return NULL; +} + /** * virStoragePoolCreateXML: * @conn: pointer to hypervisor connection diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms index 0efde25a7f..95df3a0dbc 100644 --- a/src/libvirt_public.syms +++ b/src/libvirt_public.syms @@ -779,4 +779,10 @@ LIBVIRT_3.9.0 { global: virDomainSetLifecycleAction; } LIBVIRT_3.7.0; + +LIBVIRT_4.1.0 { + global: + virStoragePoolLookupByTargetPath; +} LIBVIRT_3.9.0; + # .... define new API here using predicted next version number .... diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index ea8b275601..68e0acb2c0 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -329,7 +329,7 @@ qemuMigrationPrecreateDisk(virConnectPtr conn, *volName = '\0'; volName++; - if (!(pool = storagePoolLookupByTargetPath(conn, basePath))) + if (!(pool = virStoragePoolLookupByTargetPath(conn, basePath))) goto cleanup; format = virStorageFileFormatTypeToString(disk->src->format); if (disk->src->format == VIR_STORAGE_FILE_QCOW2) diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c index f8fa64af99..9ea726dc45 100644 --- a/src/remote/remote_driver.c +++ b/src/remote/remote_driver.c @@ -8556,6 +8556,7 @@ static virStorageDriver storage_driver = { .storagePoolLookupByName = remoteStoragePoolLookupByName, /* 0.4.1 */ .storagePoolLookupByUUID = remoteStoragePoolLookupByUUID, /* 0.4.1 */ .storagePoolLookupByVolume = remoteStoragePoolLookupByVolume, /* 0.4.1 */ + .storagePoolLookupByTargetPath = remoteStoragePoolLookupByTargetPath, /* 4.1.0 */ .storagePoolCreateXML = remoteStoragePoolCreateXML, /* 0.4.1 */ .storagePoolDefineXML = remoteStoragePoolDefineXML, /* 0.4.1 */ .storagePoolBuild = remoteStoragePoolBuild, /* 0.4.1 */ diff --git a/src/remote/remote_protocol.x b/src/remote/remote_protocol.x index 0aed25220d..9dbd497b2f 100644 --- a/src/remote/remote_protocol.x +++ b/src/remote/remote_protocol.x @@ -1761,6 +1761,14 @@ struct remote_storage_pool_lookup_by_volume_ret { remote_nonnull_storage_pool pool; }; +struct remote_storage_pool_lookup_by_target_path_args { + remote_nonnull_string path; +}; + +struct remote_storage_pool_lookup_by_target_path_ret { + remote_nonnull_storage_pool pool; +}; + struct remote_storage_pool_create_xml_args { remote_nonnull_string xml; unsigned int flags; @@ -6120,5 +6128,12 @@ enum remote_procedure { * @generate: both * @acl: domain:write */ - REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390 + REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390, + + /** + * @generate: both + * @priority: high + * @acl: storage_pool:getattr + */ + REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391 }; diff --git a/src/remote_protocol-structs b/src/remote_protocol-structs index 59b0acec69..f45aba27a2 100644 --- a/src/remote_protocol-structs +++ b/src/remote_protocol-structs @@ -1330,6 +1330,12 @@ struct remote_storage_pool_lookup_by_volume_args { struct remote_storage_pool_lookup_by_volume_ret { remote_nonnull_storage_pool pool; }; +struct remote_storage_pool_lookup_by_target_path_args { + remote_nonnull_string path; +}; +struct remote_storage_pool_lookup_by_target_path_ret { + remote_nonnull_storage_pool pool; +}; struct remote_storage_pool_create_xml_args { remote_nonnull_string xml; u_int flags; @@ -3262,4 +3268,5 @@ enum remote_procedure { REMOTE_PROC_DOMAIN_MANAGED_SAVE_GET_XML_DESC = 388, REMOTE_PROC_DOMAIN_MANAGED_SAVE_DEFINE_XML = 389, REMOTE_PROC_DOMAIN_SET_LIFECYCLE_ACTION = 390, + REMOTE_PROC_STORAGE_POOL_LOOKUP_BY_TARGET_PATH = 391, }; diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index 1b4bce4fc8..d5e38af5aa 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -1694,6 +1694,9 @@ storagePoolLookupByTargetPath(virConnectPtr conn, storagePoolLookupByTargetPathCallback, cleanpath))) { def = virStoragePoolObjGetDef(obj); + if (virStoragePoolLookupByTargetPathEnsureACL(conn, def) < 0) + goto cleanup; + pool = virGetStoragePool(conn, def->name, def->uuid, NULL, NULL); virStoragePoolObjEndAPI(&obj); } @@ -1710,6 +1713,7 @@ storagePoolLookupByTargetPath(virConnectPtr conn, } } + cleanup: VIR_FREE(cleanpath); return pool; } @@ -2808,6 +2812,7 @@ static virStorageDriver storageDriver = { .storagePoolLookupByName = storagePoolLookupByName, /* 0.4.0 */ .storagePoolLookupByUUID = storagePoolLookupByUUID, /* 0.4.0 */ .storagePoolLookupByVolume = storagePoolLookupByVolume, /* 0.4.0 */ + .storagePoolLookupByTargetPath = storagePoolLookupByTargetPath, /* 4.1.0 */ .storagePoolCreateXML = storagePoolCreateXML, /* 0.4.0 */ .storagePoolDefineXML = storagePoolDefineXML, /* 0.4.0 */ .storagePoolBuild = storagePoolBuild, /* 0.4.0 */ -- 2.14.3
The dlopened modules we currently build all use various symbols from libvirt.so, but don't actually link to it. They rely on the libvirtd daemon re-exporting the libvirt.so symbols. This means that at the time the modules are linked, they contain a huge number of undefined symbols. It also means that these undefined symbols are not versioned, so despite us providing a LIBVIRT_PRIVATE_XXXX version that intentionally changes on every release, the loadable modules could actually be loaded into any libvirtd regardless of version. This change explicitly links all modules against libvirt.so so that they don't rely on the re-export behave and can be fully resolved at build time. This will give us a stronger guarantee modules will actually be loadable at runtime and that we're using modules from the matched build. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- src/Makefile.am | 49 ++++++++++++++++++++++++++++--------------------- 1 file changed, 28 insertions(+), 21 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 7f9961fe55..6f07243b62 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1340,7 +1340,7 @@ noinst_LTLIBRARIES += libvirt_driver_xen_impl.la libvirt_driver_xen_la_SOURCES = libvirt_driver_xen_la_LIBADD = libvirt_driver_xen_impl.la mod_LTLIBRARIES += libvirt_driver_xen.la -libvirt_driver_xen_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_xen_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_xen_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_xen_impl_la_CFLAGS = \ @@ -1386,7 +1386,7 @@ libvirt_driver_vbox_la_SOURCES = libvirt_driver_vbox_la_LIBADD = libvirt_driver_vbox_impl.la mod_LTLIBRARIES += \ libvirt_driver_vbox.la -libvirt_driver_vbox_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_vbox_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_vbox_impl_la_CFLAGS = \ @@ -1415,7 +1415,7 @@ noinst_LTLIBRARIES += libvirt_driver_libxl_impl.la libvirt_driver_libxl_la_SOURCES = libvirt_driver_libxl_la_LIBADD = libvirt_driver_libxl_impl.la mod_LTLIBRARIES += libvirt_driver_libxl.la -libvirt_driver_libxl_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_libxl_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_libxl_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_libxl_impl_la_CFLAGS = \ @@ -1427,8 +1427,7 @@ libvirt_driver_libxl_impl_la_CFLAGS = \ $(AM_CFLAGS) libvirt_driver_libxl_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_libxl_impl_la_LIBADD = $(LIBXL_LIBS) \ - libvirt_xenconfig_libxl.la \ - libvirt_secret.la + libvirt_xenconfig_libxl.la libvirt_driver_libxl_impl_la_SOURCES = $(LIBXL_DRIVER_SOURCES) conf_DATA += libxl/libxl.conf @@ -1445,7 +1444,7 @@ noinst_LTLIBRARIES += libvirt_driver_qemu_impl.la libvirt_driver_qemu_la_SOURCES = libvirt_driver_qemu_la_LIBADD = libvirt_driver_qemu_impl.la mod_LTLIBRARIES += libvirt_driver_qemu.la -libvirt_driver_qemu_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_qemu_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_qemu_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_qemu_impl_la_CFLAGS = \ @@ -1461,7 +1460,6 @@ libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \ $(GNUTLS_LIBS) \ $(LIBNL_LIBS) \ $(LIBXML_LIBS) \ - libvirt_secret.la \ $(NULL) libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -1481,7 +1479,7 @@ noinst_LTLIBRARIES += libvirt_driver_lxc_impl.la libvirt_driver_lxc_la_SOURCES = libvirt_driver_lxc_la_LIBADD = libvirt_driver_lxc_impl.la mod_LTLIBRARIES += libvirt_driver_lxc.la -libvirt_driver_lxc_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_lxc_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_lxc_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_lxc_impl_la_CFLAGS = \ @@ -1518,7 +1516,7 @@ noinst_LTLIBRARIES += libvirt_driver_uml_impl.la libvirt_driver_uml_la_SOURCES = libvirt_driver_uml_la_LIBADD = libvirt_driver_uml_impl.la mod_LTLIBRARIES += libvirt_driver_uml.la -libvirt_driver_uml_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_uml_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_uml_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_uml_impl_la_CFLAGS = \ @@ -1590,7 +1588,7 @@ noinst_LTLIBRARIES += libvirt_driver_vz_impl.la libvirt_driver_vz_la_SOURCES = libvirt_driver_vz_la_LIBADD = libvirt_driver_vz_impl.la mod_LTLIBRARIES += libvirt_driver_vz.la -libvirt_driver_vz_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_vz_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_vz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_vz_impl_la_CFLAGS = \ -I$(srcdir)/conf \ @@ -1606,7 +1604,7 @@ noinst_LTLIBRARIES += libvirt_driver_bhyve_impl.la libvirt_driver_bhyve_la_SOURCES = libvirt_driver_bhyve_la_LIBADD = libvirt_driver_bhyve_impl.la mod_LTLIBRARIES += libvirt_driver_bhyve.la -libvirt_driver_bhyve_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_bhyve_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_bhyve_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_bhyve_impl_la_CFLAGS = \ @@ -1629,7 +1627,7 @@ noinst_LTLIBRARIES += libvirt_driver_network_impl.la libvirt_driver_network_la_SOURCES = libvirt_driver_network_la_LIBADD = libvirt_driver_network_impl.la mod_LTLIBRARIES += libvirt_driver_network.la -libvirt_driver_network_la_LIBADD += ../gnulib/lib/libgnu.la \ +libvirt_driver_network_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la \ $(LIBNL_LIBS) \ $(DBUS_LIBS) \ $(NULL) @@ -1663,7 +1661,7 @@ if WITH_UDEV libvirt_driver_interface_la_CFLAGS += $(UDEV_CFLAGS) libvirt_driver_interface_la_LIBADD += $(UDEV_LIBS) endif WITH_UDEV -libvirt_driver_interface_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_interface_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_interface_la_LDFLAGS += -module -avoid-version libvirt_driver_interface_la_SOURCES = $(INTERFACE_DRIVER_SOURCES) endif WITH_INTERFACE @@ -1674,7 +1672,7 @@ libvirt_driver_secret_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) -libvirt_driver_secret_la_LIBADD = ../gnulib/lib/libgnu.la +libvirt_driver_secret_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_secret_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) endif WITH_SECRETS @@ -1702,7 +1700,7 @@ noinst_LTLIBRARIES += libvirt_driver_storage_impl.la libvirt_driver_storage_la_SOURCES = libvirt_driver_storage_la_LIBADD = libvirt_driver_storage_impl.la mod_LTLIBRARIES += libvirt_driver_storage.la -libvirt_driver_storage_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_storage_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_storage_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SOURCES) @@ -1715,6 +1713,7 @@ libvirt_storage_backend_fs_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_fs.la libvirt_storage_backend_fs_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_fs_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE if WITH_STORAGE_LVM @@ -1727,6 +1726,7 @@ libvirt_storage_backend_logical_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_logical.la libvirt_storage_backend_logical_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_logical_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_LVM if WITH_STORAGE_ISCSI @@ -1740,6 +1740,7 @@ libvirt_storage_backend_iscsi_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_iscsi.la libvirt_storage_backend_iscsi_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_iscsi_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_ISCSI if WITH_STORAGE_SCSI @@ -1751,6 +1752,7 @@ libvirt_storage_backend_scsi_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_scsi.la libvirt_storage_backend_scsi_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_scsi_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_SCSI if WITH_STORAGE_MPATH @@ -1765,6 +1767,7 @@ libvirt_storage_backend_mpath_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_mpath.la libvirt_storage_backend_mpath_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_mpath_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_MPATH if WITH_STORAGE_DISK @@ -1776,6 +1779,7 @@ libvirt_storage_backend_disk_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_disk.la libvirt_storage_backend_disk_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_disk_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_DISK if WITH_STORAGE_RBD @@ -1789,6 +1793,7 @@ libvirt_storage_backend_rbd_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_rbd.la libvirt_storage_backend_rbd_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_rbd_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_RBD if WITH_STORAGE_SHEEPDOG @@ -1809,6 +1814,7 @@ noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog_priv.la storagebackend_LTLIBRARIES += libvirt_storage_backend_sheepdog.la libvirt_storage_backend_sheepdog_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_sheepdog_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_SHEEPDOG if WITH_STORAGE_GLUSTER @@ -1823,6 +1829,7 @@ libvirt_storage_backend_gluster_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_gluster.la libvirt_storage_backend_gluster_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_gluster_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_GLUSTER if WITH_STORAGE_ZFS @@ -1834,6 +1841,7 @@ libvirt_storage_backend_zfs_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_zfs.la libvirt_storage_backend_zfs_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_zfs_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_ZFS if WITH_STORAGE_VSTORAGE @@ -1846,6 +1854,7 @@ libvirt_storage_backend_vstorage_la_CFLAGS = \ storagebackend_LTLIBRARIES += libvirt_storage_backend_vstorage.la libvirt_storage_backend_vstorage_la_LDFLAGS = \ -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_vstorage_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_VSTORAGE if WITH_NODE_DEVICES @@ -1873,7 +1882,7 @@ libvirt_driver_nodedev_la_LIBADD += $(UDEV_LIBS) $(PCIACCESS_LIBS) endif WITH_UDEV endif WITH_LIBVIRTD -libvirt_driver_nodedev_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_nodedev_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_nodedev_la_LDFLAGS += -module -avoid-version endif WITH_NODE_DEVICES @@ -1896,7 +1905,7 @@ libvirt_driver_nwfilter_impl_la_LIBADD = \ $(LIBPCAP_LIBS) \ $(LIBNL_LIBS) \ $(DBUS_LIBS) -libvirt_driver_nwfilter_impl_la_LIBADD += ../gnulib/lib/libgnu.la +libvirt_driver_nwfilter_impl_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la libvirt_driver_nwfilter_impl_la_LDFLAGS += -module -avoid-version libvirt_driver_nwfilter_impl_la_SOURCES = $(NWFILTER_DRIVER_SOURCES) endif WITH_NWFILTER @@ -2596,8 +2605,7 @@ lockd_la_CFLAGS = -I$(srcdir)/conf \ $(XDR_CFLAGS) \ $(AM_CFLAGS) lockd_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) -lockd_la_LIBADD = ../gnulib/lib/libgnu.la \ - $(NULL) +lockd_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_lockd.aug if WITH_DTRACE_PROBES lockd_la_LIBADD += libvirt_probes.lo @@ -2878,8 +2886,7 @@ lockdriver_LTLIBRARIES += sanlock.la sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES) sanlock_la_CFLAGS = -I$(srcdir)/conf $(AM_CFLAGS) sanlock_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) -sanlock_la_LIBADD = -lsanlock_client \ - ../gnulib/lib/libgnu.la +sanlock_la_LIBADD = -lsanlock_client libvirt.la ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_sanlock.aug augeastest_DATA += test_libvirt_sanlock.aug -- 2.14.3
Dynamic loadable modules all need a common set of linker flags -module -avoid-version $(AM_LDFLAGS) Bundle those up into a $(AM_LDFLAGS_MOD) to avoid repetition. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- src/Makefile.am | 69 +++++++++++++++++++++++---------------------------------- 1 file changed, 28 insertions(+), 41 deletions(-) diff --git a/src/Makefile.am b/src/Makefile.am index 6f07243b62..38efde012e 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -51,6 +51,7 @@ AM_LDFLAGS = $(DRIVER_MODULES_LDFLAGS) \ $(CYGWIN_EXTRA_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) +AM_LDFLAGS_MOD = -module -avoid-version $(AM_LDFLAGS) POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" @@ -1341,7 +1342,7 @@ libvirt_driver_xen_la_SOURCES = libvirt_driver_xen_la_LIBADD = libvirt_driver_xen_impl.la mod_LTLIBRARIES += libvirt_driver_xen.la libvirt_driver_xen_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_xen_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_xen_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_xen_impl_la_CFLAGS = \ $(XEN_CFLAGS) \ @@ -1387,7 +1388,7 @@ libvirt_driver_vbox_la_LIBADD = libvirt_driver_vbox_impl.la mod_LTLIBRARIES += \ libvirt_driver_vbox.la libvirt_driver_vbox_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_vbox_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_vbox_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_vbox_impl_la_CFLAGS = \ -I$(srcdir)/conf \ @@ -1416,7 +1417,7 @@ libvirt_driver_libxl_la_SOURCES = libvirt_driver_libxl_la_LIBADD = libvirt_driver_libxl_impl.la mod_LTLIBRARIES += libvirt_driver_libxl.la libvirt_driver_libxl_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_libxl_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_libxl_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_libxl_impl_la_CFLAGS = \ $(LIBXL_CFLAGS) \ @@ -1445,7 +1446,7 @@ libvirt_driver_qemu_la_SOURCES = libvirt_driver_qemu_la_LIBADD = libvirt_driver_qemu_impl.la mod_LTLIBRARIES += libvirt_driver_qemu.la libvirt_driver_qemu_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_qemu_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_qemu_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_qemu_impl_la_CFLAGS = \ $(GNUTLS_CFLAGS) \ @@ -1480,7 +1481,7 @@ libvirt_driver_lxc_la_SOURCES = libvirt_driver_lxc_la_LIBADD = libvirt_driver_lxc_impl.la mod_LTLIBRARIES += libvirt_driver_lxc.la libvirt_driver_lxc_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_lxc_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_lxc_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_lxc_impl_la_CFLAGS = \ $(LIBNL_CFLAGS) \ @@ -1517,7 +1518,7 @@ libvirt_driver_uml_la_SOURCES = libvirt_driver_uml_la_LIBADD = libvirt_driver_uml_impl.la mod_LTLIBRARIES += libvirt_driver_uml.la libvirt_driver_uml_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_uml_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_uml_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_uml_impl_la_CFLAGS = \ -I$(srcdir)/access \ @@ -1589,7 +1590,7 @@ libvirt_driver_vz_la_SOURCES = libvirt_driver_vz_la_LIBADD = libvirt_driver_vz_impl.la mod_LTLIBRARIES += libvirt_driver_vz.la libvirt_driver_vz_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_vz_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_vz_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_vz_impl_la_CFLAGS = \ -I$(srcdir)/conf \ -I$(srcdir)/access \ @@ -1605,7 +1606,7 @@ libvirt_driver_bhyve_la_SOURCES = libvirt_driver_bhyve_la_LIBADD = libvirt_driver_bhyve_impl.la mod_LTLIBRARIES += libvirt_driver_bhyve.la libvirt_driver_bhyve_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_bhyve_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_bhyve_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_bhyve_impl_la_CFLAGS = \ -I$(srcdir)/access \ @@ -1631,7 +1632,7 @@ libvirt_driver_network_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la \ $(LIBNL_LIBS) \ $(DBUS_LIBS) \ $(NULL) -libvirt_driver_network_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_network_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_network_impl_la_CFLAGS = \ $(LIBNL_CFLAGS) \ @@ -1651,7 +1652,7 @@ libvirt_driver_interface_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) $(LIBNL_CFLAGS) -libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_interface_la_LIBADD = if WITH_NETCF libvirt_driver_interface_la_CFLAGS += $(NETCF_CFLAGS) @@ -1662,7 +1663,6 @@ libvirt_driver_interface_la_CFLAGS += $(UDEV_CFLAGS) libvirt_driver_interface_la_LIBADD += $(UDEV_LIBS) endif WITH_UDEV libvirt_driver_interface_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_interface_la_LDFLAGS += -module -avoid-version libvirt_driver_interface_la_SOURCES = $(INTERFACE_DRIVER_SOURCES) endif WITH_INTERFACE @@ -1673,7 +1673,7 @@ libvirt_driver_secret_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) libvirt_driver_secret_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_secret_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_secret_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) endif WITH_SECRETS @@ -1701,7 +1701,7 @@ libvirt_driver_storage_la_SOURCES = libvirt_driver_storage_la_LIBADD = libvirt_driver_storage_impl.la mod_LTLIBRARIES += libvirt_driver_storage.la libvirt_driver_storage_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_storage_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_storage_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SOURCES) @@ -1711,8 +1711,7 @@ libvirt_storage_backend_fs_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_fs.la -libvirt_storage_backend_fs_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_fs_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_fs_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE @@ -1724,8 +1723,7 @@ libvirt_storage_backend_logical_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_logical.la -libvirt_storage_backend_logical_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_logical_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_logical_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_LVM @@ -1738,8 +1736,7 @@ libvirt_storage_backend_iscsi_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_iscsi.la -libvirt_storage_backend_iscsi_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_iscsi_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_iscsi_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_ISCSI @@ -1750,8 +1747,7 @@ libvirt_storage_backend_scsi_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_scsi.la -libvirt_storage_backend_scsi_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_scsi_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_scsi_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_SCSI @@ -1765,8 +1761,7 @@ libvirt_storage_backend_mpath_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_mpath.la -libvirt_storage_backend_mpath_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_mpath_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_mpath_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_MPATH @@ -1777,8 +1772,7 @@ libvirt_storage_backend_disk_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_disk.la -libvirt_storage_backend_disk_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_disk_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_disk_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_DISK @@ -1791,8 +1785,7 @@ libvirt_storage_backend_rbd_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_rbd.la -libvirt_storage_backend_rbd_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_rbd_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_rbd_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_RBD @@ -1812,8 +1805,7 @@ libvirt_storage_backend_sheepdog_priv_la_CFLAGS = \ noinst_LTLIBRARIES += libvirt_storage_backend_sheepdog_priv.la storagebackend_LTLIBRARIES += libvirt_storage_backend_sheepdog.la -libvirt_storage_backend_sheepdog_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_sheepdog_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_sheepdog_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_SHEEPDOG @@ -1827,8 +1819,7 @@ libvirt_storage_backend_gluster_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_gluster.la -libvirt_storage_backend_gluster_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_gluster_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_gluster_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_GLUSTER @@ -1839,8 +1830,7 @@ libvirt_storage_backend_zfs_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_zfs.la -libvirt_storage_backend_zfs_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_zfs_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_zfs_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_ZFS @@ -1852,8 +1842,7 @@ libvirt_storage_backend_vstorage_la_CFLAGS = \ $(AM_CFLAGS) storagebackend_LTLIBRARIES += libvirt_storage_backend_vstorage.la -libvirt_storage_backend_vstorage_la_LDFLAGS = \ - -module -avoid-version $(AM_LDFLAGS) +libvirt_storage_backend_vstorage_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_storage_backend_vstorage_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la endif WITH_STORAGE_VSTORAGE @@ -1866,7 +1855,7 @@ libvirt_driver_nodedev_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) $(LIBNL_CFLAGS) -libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS) +libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_nodedev_la_LIBADD = if WITH_LIBVIRTD @@ -1883,7 +1872,6 @@ endif WITH_UDEV endif WITH_LIBVIRTD libvirt_driver_nodedev_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_nodedev_la_LDFLAGS += -module -avoid-version endif WITH_NODE_DEVICES @@ -1892,7 +1880,7 @@ noinst_LTLIBRARIES += libvirt_driver_nwfilter_impl.la libvirt_driver_nwfilter_la_SOURCES = libvirt_driver_nwfilter_la_LIBADD = libvirt_driver_nwfilter_impl.la mod_LTLIBRARIES += libvirt_driver_nwfilter.la -libvirt_driver_nwfilter_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +libvirt_driver_nwfilter_la_LDFLAGS = $(AM_LDFLAGS_MOD) libvirt_driver_nwfilter_impl_la_CFLAGS = \ $(LIBPCAP_CFLAGS) \ $(LIBNL_CFLAGS) \ @@ -1906,7 +1894,6 @@ libvirt_driver_nwfilter_impl_la_LIBADD = \ $(LIBNL_LIBS) \ $(DBUS_LIBS) libvirt_driver_nwfilter_impl_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_nwfilter_impl_la_LDFLAGS += -module -avoid-version libvirt_driver_nwfilter_impl_la_SOURCES = $(NWFILTER_DRIVER_SOURCES) endif WITH_NWFILTER @@ -2604,7 +2591,7 @@ lockd_la_SOURCES = \ lockd_la_CFLAGS = -I$(srcdir)/conf \ $(XDR_CFLAGS) \ $(AM_CFLAGS) -lockd_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +lockd_la_LDFLAGS = $(AM_LDFLAGS_MOD) lockd_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_lockd.aug if WITH_DTRACE_PROBES @@ -2885,7 +2872,7 @@ if WITH_SANLOCK lockdriver_LTLIBRARIES += sanlock.la sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES) sanlock_la_CFLAGS = -I$(srcdir)/conf $(AM_CFLAGS) -sanlock_la_LDFLAGS = -module -avoid-version $(AM_LDFLAGS) +sanlock_la_LDFLAGS = $(AM_LDFLAGS_MOD) sanlock_la_LIBADD = -lsanlock_client libvirt.la ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_sanlock.aug -- 2.14.3
Undefined symbols are a bad thing in general because they can get resolved in unexpected ways at runtime if multiple sources provide the same symbol name. For example both glibc and libtirpc may provide XDR symbols and we want to ensure that we resolve to libtirpc if that's what we originally built against. The toolchain maintainers thus strongly recommend that all applications use the '-z defs' linker flag to prevent undefined symbols. This is shortly becoming part of the default linker flags for RPMs. As an added benefit this aligns Linux builds with Windows builds, where the linker has never permitted undefined symbols. Signed-off-by: Daniel P. Berrange <berrange@redhat.com> --- configure.ac | 1 + daemon/Makefile.am | 2 ++ m4/virt-linker-no-undefined.m4 | 32 ++++++++++++++++++++++++++++++++ src/Makefile.am | 40 ++++++++++++++++++++++++---------------- tools/Makefile.am | 1 + 5 files changed, 60 insertions(+), 16 deletions(-) create mode 100644 m4/virt-linker-no-undefined.m4 diff --git a/configure.ac b/configure.ac index 4cccf7f4de..7997ec5a14 100644 --- a/configure.ac +++ b/configure.ac @@ -237,6 +237,7 @@ LIBVIRT_COMPILE_WARNINGS LIBVIRT_COMPILE_PIE LIBVIRT_LINKER_RELRO LIBVIRT_LINKER_NO_INDIRECT +LIBVIRT_LINKER_NO_UNDEFINED LIBVIRT_ARG_APPARMOR LIBVIRT_ARG_ATTR diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 9c1dfcfac6..77dfd8943a 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -125,6 +125,7 @@ libvirtd_conf_la_LDFLAGS = \ $(PIE_LDFLAGS) \ $(COVERAGE_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ + $(NO_UNDEFINED_LDFLAGS) \ $(NULL) libvirtd_conf_la_LIBADD = $(LIBXML_LIBS) @@ -158,6 +159,7 @@ libvirtd_LDFLAGS = \ $(PIE_LDFLAGS) \ $(COVERAGE_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ + $(NO_UNDEFINED_LDFLAGS) \ $(NULL) libvirtd_LDADD = \ diff --git a/m4/virt-linker-no-undefined.m4 b/m4/virt-linker-no-undefined.m4 new file mode 100644 index 0000000000..532b0de212 --- /dev/null +++ b/m4/virt-linker-no-undefined.m4 @@ -0,0 +1,32 @@ +dnl +dnl Check for -z defs linker flag +dnl +dnl Copyright (C) 2013-2018 Red Hat, Inc. +dnl +dnl This library is free software; you can redistribute it and/or +dnl modify it under the terms of the GNU Lesser General Public +dnl License as published by the Free Software Foundation; either +dnl version 2.1 of the License, or (at your option) any later version. +dnl +dnl This library is distributed in the hope that it will be useful, +dnl but WITHOUT ANY WARRANTY; without even the implied warranty of +dnl MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU +dnl Lesser General Public License for more details. +dnl +dnl You should have received a copy of the GNU Lesser General Public +dnl License along with this library. If not, see +dnl <http://www.gnu.org/licenses/>. +dnl + +AC_DEFUN([LIBVIRT_LINKER_NO_UNDEFINED],[ + AC_MSG_CHECKING([for how to stop undefined symbols at link time]) + + NO_UNDEFINED_LDFLAGS= + ld_help=`$LD --help 2>&1` + case $ld_help in + *"-z defs"*) NO_UNDEFINED_LDFLAGS="-Wl,-z -Wl,defs" ;; + esac + AC_SUBST([NO_UNDEFINED_LDFLAGS]) + + AC_MSG_RESULT([$NO_UNDEFINED_LDFLAGS]) +]) diff --git a/src/Makefile.am b/src/Makefile.am index 38efde012e..2cfa36a142 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -52,6 +52,7 @@ AM_LDFLAGS = $(DRIVER_MODULES_LDFLAGS) \ $(MINGW_EXTRA_LDFLAGS) \ $(NULL) AM_LDFLAGS_MOD = -module -avoid-version $(AM_LDFLAGS) +AM_LDFLAGS_MOD_NOUNDEF = $(AM_LDFLAGS_MOD) $(NO_UNDEFINED_LDFLAGS) POD2MAN = pod2man -c "Virtualization Support" -r "$(PACKAGE)-$(VERSION)" @@ -1342,7 +1343,7 @@ libvirt_driver_xen_la_SOURCES = libvirt_driver_xen_la_LIBADD = libvirt_driver_xen_impl.la mod_LTLIBRARIES += libvirt_driver_xen.la libvirt_driver_xen_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_xen_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_xen_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_xen_impl_la_CFLAGS = \ $(XEN_CFLAGS) \ @@ -1388,7 +1389,7 @@ libvirt_driver_vbox_la_LIBADD = libvirt_driver_vbox_impl.la mod_LTLIBRARIES += \ libvirt_driver_vbox.la libvirt_driver_vbox_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_vbox_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_vbox_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_vbox_impl_la_CFLAGS = \ -I$(srcdir)/conf \ @@ -1417,10 +1418,11 @@ libvirt_driver_libxl_la_SOURCES = libvirt_driver_libxl_la_LIBADD = libvirt_driver_libxl_impl.la mod_LTLIBRARIES += libvirt_driver_libxl.la libvirt_driver_libxl_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_libxl_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_libxl_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_libxl_impl_la_CFLAGS = \ $(LIBXL_CFLAGS) \ + $(LIBXML_CFLAGS) \ -I$(srcdir)/access \ -I$(srcdir)/conf \ -I$(srcdir)/secret \ @@ -1428,6 +1430,7 @@ libvirt_driver_libxl_impl_la_CFLAGS = \ $(AM_CFLAGS) libvirt_driver_libxl_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_libxl_impl_la_LIBADD = $(LIBXL_LIBS) \ + $(LIBXML_LIBS) \ libvirt_xenconfig_libxl.la libvirt_driver_libxl_impl_la_SOURCES = $(LIBXL_DRIVER_SOURCES) @@ -1446,11 +1449,12 @@ libvirt_driver_qemu_la_SOURCES = libvirt_driver_qemu_la_LIBADD = libvirt_driver_qemu_impl.la mod_LTLIBRARIES += libvirt_driver_qemu.la libvirt_driver_qemu_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_qemu_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_qemu_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_qemu_impl_la_CFLAGS = \ $(GNUTLS_CFLAGS) \ $(LIBNL_CFLAGS) \ + $(SELINUX_CFLAGS) \ $(XDR_CFLAGS) \ -I$(srcdir)/access \ -I$(srcdir)/conf \ @@ -1460,6 +1464,7 @@ libvirt_driver_qemu_impl_la_LDFLAGS = $(AM_LDFLAGS) libvirt_driver_qemu_impl_la_LIBADD = $(CAPNG_LIBS) \ $(GNUTLS_LIBS) \ $(LIBNL_LIBS) \ + $(SELINUX_LIBS) \ $(LIBXML_LIBS) \ $(NULL) libvirt_driver_qemu_impl_la_SOURCES = $(QEMU_DRIVER_SOURCES) @@ -1481,7 +1486,7 @@ libvirt_driver_lxc_la_SOURCES = libvirt_driver_lxc_la_LIBADD = libvirt_driver_lxc_impl.la mod_LTLIBRARIES += libvirt_driver_lxc.la libvirt_driver_lxc_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_lxc_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_lxc_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_lxc_impl_la_CFLAGS = \ $(LIBNL_CFLAGS) \ @@ -1518,7 +1523,7 @@ libvirt_driver_uml_la_SOURCES = libvirt_driver_uml_la_LIBADD = libvirt_driver_uml_impl.la mod_LTLIBRARIES += libvirt_driver_uml.la libvirt_driver_uml_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_uml_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_uml_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_uml_impl_la_CFLAGS = \ -I$(srcdir)/access \ @@ -1590,7 +1595,7 @@ libvirt_driver_vz_la_SOURCES = libvirt_driver_vz_la_LIBADD = libvirt_driver_vz_impl.la mod_LTLIBRARIES += libvirt_driver_vz.la libvirt_driver_vz_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_vz_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_vz_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_vz_impl_la_CFLAGS = \ -I$(srcdir)/conf \ -I$(srcdir)/access \ @@ -1606,7 +1611,7 @@ libvirt_driver_bhyve_la_SOURCES = libvirt_driver_bhyve_la_LIBADD = libvirt_driver_bhyve_impl.la mod_LTLIBRARIES += libvirt_driver_bhyve.la libvirt_driver_bhyve_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_bhyve_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_bhyve_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_bhyve_impl_la_CFLAGS = \ -I$(srcdir)/access \ @@ -1632,7 +1637,7 @@ libvirt_driver_network_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la \ $(LIBNL_LIBS) \ $(DBUS_LIBS) \ $(NULL) -libvirt_driver_network_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_network_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_network_impl_la_CFLAGS = \ $(LIBNL_CFLAGS) \ @@ -1652,7 +1657,7 @@ libvirt_driver_interface_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) $(LIBNL_CFLAGS) -libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_interface_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_interface_la_LIBADD = if WITH_NETCF libvirt_driver_interface_la_CFLAGS += $(NETCF_CFLAGS) @@ -1673,7 +1678,7 @@ libvirt_driver_secret_la_CFLAGS = \ -I$(srcdir)/conf \ $(AM_CFLAGS) libvirt_driver_secret_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_secret_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_secret_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_secret_la_SOURCES = $(SECRET_DRIVER_SOURCES) endif WITH_SECRETS @@ -1701,7 +1706,7 @@ libvirt_driver_storage_la_SOURCES = libvirt_driver_storage_la_LIBADD = libvirt_driver_storage_impl.la mod_LTLIBRARIES += libvirt_driver_storage.la libvirt_driver_storage_la_LIBADD += libvirt.la ../gnulib/lib/libgnu.la -libvirt_driver_storage_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_storage_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_storage_impl_la_SOURCES += $(STORAGE_DRIVER_SOURCES) @@ -1855,7 +1860,7 @@ libvirt_driver_nodedev_la_CFLAGS = \ -I$(srcdir)/access \ -I$(srcdir)/conf \ $(AM_CFLAGS) $(LIBNL_CFLAGS) -libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_nodedev_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_nodedev_la_LIBADD = if WITH_LIBVIRTD @@ -1880,7 +1885,7 @@ noinst_LTLIBRARIES += libvirt_driver_nwfilter_impl.la libvirt_driver_nwfilter_la_SOURCES = libvirt_driver_nwfilter_la_LIBADD = libvirt_driver_nwfilter_impl.la mod_LTLIBRARIES += libvirt_driver_nwfilter.la -libvirt_driver_nwfilter_la_LDFLAGS = $(AM_LDFLAGS_MOD) +libvirt_driver_nwfilter_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) libvirt_driver_nwfilter_impl_la_CFLAGS = \ $(LIBPCAP_CFLAGS) \ $(LIBNL_CFLAGS) \ @@ -2389,6 +2394,7 @@ libvirt_la_LDFLAGS = \ $(VERSION_SCRIPT_FLAGS)$(LIBVIRT_SYMBOL_FILE) \ -version-info $(LIBVIRT_VERSION_INFO) \ $(LIBVIRT_NODELETE) \ + $(NO_UNDEFINED_LDFLAGS) \ $(AM_LDFLAGS) \ $(NULL) libvirt_la_BUILT_LIBADD += ../gnulib/lib/libgnu.la @@ -2591,7 +2597,7 @@ lockd_la_SOURCES = \ lockd_la_CFLAGS = -I$(srcdir)/conf \ $(XDR_CFLAGS) \ $(AM_CFLAGS) -lockd_la_LDFLAGS = $(AM_LDFLAGS_MOD) +lockd_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) lockd_la_LIBADD = libvirt.la ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_lockd.aug if WITH_DTRACE_PROBES @@ -2631,6 +2637,7 @@ virtlockd_CFLAGS = \ virtlockd_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ + $(NO_UNDEFINED_LDFLAGS) \ $(NULL) virtlockd_LDADD = \ libvirt_driver_admin.la \ @@ -2658,6 +2665,7 @@ virtlogd_CFLAGS = \ virtlogd_LDFLAGS = \ $(AM_LDFLAGS) \ $(PIE_LDFLAGS) \ + $(NO_UNDEFINED_LDFLAGS) \ $(NULL) virtlogd_LDADD = \ libvirt_driver_admin.la \ @@ -2872,7 +2880,7 @@ if WITH_SANLOCK lockdriver_LTLIBRARIES += sanlock.la sanlock_la_SOURCES = $(LOCK_DRIVER_SANLOCK_SOURCES) sanlock_la_CFLAGS = -I$(srcdir)/conf $(AM_CFLAGS) -sanlock_la_LDFLAGS = $(AM_LDFLAGS_MOD) +sanlock_la_LDFLAGS = $(AM_LDFLAGS_MOD_NOUNDEF) sanlock_la_LIBADD = -lsanlock_client libvirt.la ../gnulib/lib/libgnu.la augeas_DATA += locking/libvirt_sanlock.aug diff --git a/tools/Makefile.am b/tools/Makefile.am index 4c33e78a1d..85e640b9e7 100644 --- a/tools/Makefile.am +++ b/tools/Makefile.am @@ -35,6 +35,7 @@ AM_CFLAGS = \ AM_LDFLAGS = \ $(RELRO_LDFLAGS) \ $(NO_INDIRECT_LDFLAGS) \ + $(NO_UNDEFINED_LDFLAGS) \ $(NULL) ICON_FILES = \ -- 2.14.3
Loadable drivers must never depend on each other. Over time some usage mistakenly crept in for the storage and network drivers, but now this is eliminated the syntax-check rules can enforce this separation once more. Signed-off-by: Daniel P. Berrangé <berrange@redhat.com> --- cfg.mk | 2 +- src/qemu/qemu_command.c | 1 - src/qemu/qemu_domain.c | 2 -- src/qemu/qemu_migration.c | 1 - src/vz/vz_sdk.c | 1 - 5 files changed, 1 insertion(+), 6 deletions(-) diff --git a/cfg.mk b/cfg.mk index c26556fb2f..78f805b27e 100644 --- a/cfg.mk +++ b/cfg.mk @@ -769,7 +769,7 @@ sc_prohibit_gettext_markup: # lower-level code must not include higher-level headers. cross_dirs=$(patsubst $(srcdir)/src/%.,%,$(wildcard $(srcdir)/src/*/.)) cross_dirs_re=($(subst / ,/|,$(cross_dirs))) -mid_dirs=access|admin|conf|cpu|locking|logging|network|node_device|rpc|security|storage +mid_dirs=access|admin|conf|cpu|locking|logging|rpc|security sc_prohibit_cross_inclusion: @for dir in $(cross_dirs); do \ case $$dir in \ diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 5432700287..907f5c9b38 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -51,7 +51,6 @@ #include "snapshot_conf.h" #include "storage_conf.h" #include "secret_conf.h" -#include "network/bridge_driver.h" #include "virnetdevtap.h" #include "virnetdevopenvswitch.h" #include "device_conf.h" diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index f6d2723a8a..f16bc79031 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -55,8 +55,6 @@ #include "logging/log_manager.h" #include "locking/domain_lock.h" -#include "storage/storage_driver.h" - #ifdef MAJOR_IN_MKDEV # include <sys/mkdev.h> #elif MAJOR_IN_SYSMACROS diff --git a/src/qemu/qemu_migration.c b/src/qemu/qemu_migration.c index 68e0acb2c0..fdb240bce8 100644 --- a/src/qemu/qemu_migration.c +++ b/src/qemu/qemu_migration.c @@ -58,7 +58,6 @@ #include "virtypedparam.h" #include "virprocess.h" #include "nwfilter_conf.h" -#include "storage/storage_driver.h" #define VIR_FROM_THIS VIR_FROM_QEMU diff --git a/src/vz/vz_sdk.c b/src/vz/vz_sdk.c index eea5f6fc68..41b09162c3 100644 --- a/src/vz/vz_sdk.c +++ b/src/vz/vz_sdk.c @@ -33,7 +33,6 @@ #include "virhostcpu.h" #include "virsocketaddr.h" -#include "storage/storage_driver.h" #include "vz_sdk.h" #define VIR_FROM_THIS VIR_FROM_PARALLELS -- 2.14.3
On 02/05/2018 04:28 PM, Daniel P. Berrangé wrote:
This was triggered by the recent Fedora change to add '-z defs' to RPM builds by default which breaks libvirt. Various make rule changes can fix much of the problem, but it also requires source refactoring to get rid of places where virt drivers directly call into the storage/network drivers. Co-incidentally this work will also be useful in allowing us to separate out drivers to distinct daemons.
In v3:
- Fixed a few build problems identified by travis
In v2:
- Fixed header file name comment - Resolve conflicts - Fix unit tests - Fix bisectable build by moving libvirt_lxc build patch earlier - Update syntax check header include rule
Daniel P. Berrangé (15): storage: extract storage file backend from main storage driver backend storage: move storage file backend framework into util directory rpc: don't link in second copy of RPC code to libvirtd & lockd plugin build: link libvirt_lxc against libvirt.so conf: introduce callback registration for domain net device allocation conf: expand network device callbacks to cover bandwidth updates qemu: replace networkGetNetworkAddress with public API calls conf: expand network device callbacks to cover resolving NIC type network: remove conditional declarations conf: move virStorageTranslateDiskSourcePool into domain conf storage: export virStoragePoolLookupByTargetPath as a public API build: explicitly link all modules with libvirt.so build: provide a AM_FLAGS_MOD for loadable modules build: passing the "-z defs" linker flag to prevent undefined symbols cfg: forbid includes of headers in network and storage drivers again
cfg.mk | 2 +- configure.ac | 1 + daemon/Makefile.am | 3 +- include/libvirt/libvirt-storage.h | 2 + m4/virt-linker-no-undefined.m4 | 32 ++ po/POTFILES.in | 2 +- src/Makefile.am | 150 ++++---- src/bhyve/bhyve_command.c | 7 +- src/conf/domain_conf.c | 355 +++++++++++++++++++ src/conf/domain_conf.h | 71 ++++ src/driver-storage.h | 5 + src/libvirt-storage.c | 40 +++ src/libvirt_private.syms | 29 ++ src/libvirt_public.syms | 6 + src/libvirt_remote.syms | 11 +- src/libxl/libxl_domain.c | 5 +- src/libxl/libxl_driver.c | 7 +- src/lxc/lxc_driver.c | 5 +- src/lxc/lxc_process.c | 7 +- src/network/bridge_driver.c | 124 +------ src/network/bridge_driver.h | 72 ---- src/qemu/qemu_alias.c | 3 +- src/qemu/qemu_command.c | 1 - src/qemu/qemu_domain.c | 3 - src/qemu/qemu_domain_address.c | 3 +- src/qemu/qemu_driver.c | 15 +- src/qemu/qemu_hotplug.c | 18 +- src/qemu/qemu_migration.c | 3 +- src/qemu/qemu_process.c | 115 +++++- src/remote/remote_driver.c | 1 + src/remote/remote_protocol.x | 17 +- src/remote_protocol-structs | 7 + src/security/virt-aa-helper.c | 2 - src/storage/storage_backend.c | 66 ---- src/storage/storage_backend.h | 75 ---- src/storage/storage_backend_fs.c | 8 +- src/storage/storage_backend_gluster.c | 4 +- src/storage/storage_driver.c | 256 +------------- src/storage/storage_driver.h | 3 - src/storage/storage_source.c | 645 ---------------------------------- src/storage/storage_source.h | 59 ---- src/util/virstoragefile.c | 609 +++++++++++++++++++++++++++++++- src/util/virstoragefile.h | 32 ++ src/util/virstoragefilebackend.c | 108 ++++++ src/util/virstoragefilebackend.h | 104 ++++++ src/vz/vz_sdk.c | 1 - tests/Makefile.am | 11 +- tests/qemuxml2argvtest.c | 4 + tests/virstoragetest.c | 1 - tools/Makefile.am | 1 + 50 files changed, 1679 insertions(+), 1432 deletions(-) create mode 100644 m4/virt-linker-no-undefined.m4 delete mode 100644 src/storage/storage_source.c delete mode 100644 src/storage/storage_source.h create mode 100644 src/util/virstoragefilebackend.c create mode 100644 src/util/virstoragefilebackend.h
ACK series. Michal
participants (3)
-
Daniel P. Berrangé -
John Ferlan -
Michal Privoznik