Use the full storage driver registration method that also fails if one of the storage backends is not present. This makes the test fail if a submodule fails registration, which is useful for testing. Additionally return EXIT_FAILURE as usual in tests rather than -1. --- tests/virstoragetest.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index f34408395..a73c53839 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -32,7 +32,6 @@ #include "dirname.h" #include "storage/storage_driver.h" -#include "storage/storage_backend.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -732,8 +731,8 @@ mymain(void) virStorageSourcePtr chain2; /* short for chain->backingStore */ virStorageSourcePtr chain3; /* short for chain2->backingStore */ - if (virStorageBackendDriversRegister(false) < 0) - return -1; + if (storageRegisterAll() < 0) + return EXIT_FAILURE; /* Prep some files with qemu-img; if that is not found on PATH, or * if it lacks support for qcow2 and qed, skip this test. */ -- 2.12.2

The helper methods for actually accessing the storage objects don't really belong to the main storage driver implementation file. Split them out. --- po/POTFILES.in | 1 + src/Makefile.am | 1 + src/qemu/qemu_domain.c | 1 + src/qemu/qemu_driver.c | 1 + src/security/virt-aa-helper.c | 2 +- src/storage/storage_driver.c | 551 +-------------------------------------- src/storage/storage_driver.h | 28 -- src/storage/storage_source.c | 585 ++++++++++++++++++++++++++++++++++++++++++ src/storage/storage_source.h | 53 ++++ tests/virstoragetest.c | 1 + 10 files changed, 645 insertions(+), 579 deletions(-) create mode 100644 src/storage/storage_source.c create mode 100644 src/storage/storage_source.h diff --git a/po/POTFILES.in b/po/POTFILES.in index 275df1f29..87634d228 100644 --- a/po/POTFILES.in +++ b/po/POTFILES.in @@ -185,6 +185,7 @@ 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_util.c src/test/test_driver.c src/uml/uml_conf.c diff --git a/src/Makefile.am b/src/Makefile.am index eae32dc58..399d031dd 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -1053,6 +1053,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_DRIVER_BACKEND_SOURCES) \ storage/storage_util.h storage/storage_util.c diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 8e7404da6..bc3a85ca2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -54,6 +54,7 @@ #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 e91663ca9..052c7f0fc 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -100,6 +100,7 @@ #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 7f3b7ad08..695272076 100644 --- a/src/security/virt-aa-helper.c +++ b/src/security/virt-aa-helper.c @@ -55,7 +55,7 @@ #include "virstring.h" #include "virgettext.h" -#include "storage/storage_driver.h" +#include "storage/storage_source.h" #define VIR_FROM_THIS VIR_FROM_SECURITY diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c index ab1dc8f36..ffffd5a37 100644 --- a/src/storage/storage_driver.c +++ b/src/storage/storage_driver.c @@ -50,7 +50,7 @@ #include "configmake.h" #include "virstring.h" #include "viraccessapicheck.h" -#include "dirname.h" +//#include "dirname.h" #include "storage_util.h" #define VIR_FROM_THIS VIR_FROM_STORAGE @@ -2758,555 +2758,6 @@ storageRegisterAll(void) } -/* ----------- file handlers cooperating with storage driver --------------- */ -static bool -virStorageFileIsInitialized(const virStorageSource *src) -{ - return src && src->drv; -} - - -static bool -virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) -{ - int actualType; - virStorageFileBackendPtr backend; - - if (!src) - return false; - actualType = virStorageSourceGetActualType(src); - - if (src->drv) { - backend = src->drv->backend; - } else { - if (!(backend = virStorageFileBackendForTypeInternal(actualType, - src->protocol, - false))) - return false; - } - - return backend->storageFileGetUniqueIdentifier && - backend->storageFileReadHeader && - 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) -{ - int actualType; - virStorageFileBackendPtr backend; - - if (!src) - return false; - actualType = virStorageSourceGetActualType(src); - - if (src->drv) { - backend = src->drv->backend; - } else { - if (!(backend = virStorageFileBackendForTypeInternal(actualType, - src->protocol, - false))) - return false; - } - - return !!backend->storageFileChown; -} - - -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; -} - - -/** - * virStorageFileReadHeader: read the beginning bytes of a file into a buffer - * - * @src: file structure pointing to the file - * @max_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 -virStorageFileReadHeader(virStorageSourcePtr src, - ssize_t max_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->storageFileReadHeader) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("storage file header reading is not supported for " - "storage type %s (protocol: %s)"), - virStorageTypeToString(src->type), - virStorageNetProtocolTypeToString(src->protocol)); - return -2; - } - - ret = src->drv->backend->storageFileReadHeader(src, max_len, buf); - - VIR_DEBUG("read of storage header %p: ret=%zd", src, ret); - - 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); -} - - -/* 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) -{ - 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) { - if (src == parent) { - virReportSystemError(errno, - _("Cannot access storage file '%s' " - "(as uid:%u, gid:%u)"), - src->path, (unsigned int)uid, - (unsigned int)gid); - } else { - virReportSystemError(errno, - _("Cannot access backing file '%s' " - "of storage file '%s' (as uid:%u, gid:%u)"), - src->path, parent->path, - (unsigned int)uid, (unsigned int)gid); - } - - 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 = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER, - &buf)) < 0) - goto cleanup; - - if (virStorageFileGetMetadataInternal(src, buf, headerLen, - &backingFormat) < 0) - goto cleanup; - - /* check whether we need to go deeper */ - if (!src->backingStoreRaw) { - ret = 0; - goto cleanup; - } - - 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)) < 0) { - if (report_broken) - goto cleanup; - - /* if we fail somewhere midway, just accept and return a - * broken chain */ - ret = 0; - goto cleanup; - } - - src->backingStore = backingStore; - backingStore = NULL; - ret = 0; - - cleanup: - 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; - int ret = -1; - - if (!(cycle = virHashCreate(5, NULL))) - return -1; - - if (src->format <= VIR_STORAGE_FILE_NONE) - src->format = allow_probe ? - VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; - - ret = virStorageFileGetMetadataRecurse(src, src, uid, gid, - allow_probe, report_broken, cycle); - - 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 = virStorageFileReadHeader(src, 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; -} - - static int virStorageAddISCSIPoolSourceHost(virDomainDiskDefPtr def, virStoragePoolDefPtr pooldef) diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h index ade05f715..8b913a1b8 100644 --- a/src/storage/storage_driver.h +++ b/src/storage/storage_driver.h @@ -28,34 +28,6 @@ # include "domain_conf.h" # include "virstorageobj.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 virStorageFileReadHeader(virStorageSourcePtr src, - ssize_t max_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); - -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); int virStorageTranslateDiskSourcePool(virConnectPtr conn, virDomainDiskDefPtr def); diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c new file mode 100644 index 000000000..c91d629c8 --- /dev/null +++ b/src/storage/storage_source.c @@ -0,0 +1,585 @@ +/* + * 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_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 bool +virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src) +{ + int actualType; + virStorageFileBackendPtr backend; + + if (!src) + return false; + actualType = virStorageSourceGetActualType(src); + + if (src->drv) { + backend = src->drv->backend; + } else { + if (!(backend = virStorageFileBackendForTypeInternal(actualType, + src->protocol, + false))) + return false; + } + + return backend->storageFileGetUniqueIdentifier && + backend->storageFileReadHeader && + 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) +{ + int actualType; + virStorageFileBackendPtr backend; + + if (!src) + return false; + actualType = virStorageSourceGetActualType(src); + + if (src->drv) { + backend = src->drv->backend; + } else { + if (!(backend = virStorageFileBackendForTypeInternal(actualType, + src->protocol, + false))) + return false; + } + + return !!backend->storageFileChown; +} + + +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; +} + + +/** + * virStorageFileReadHeader: read the beginning bytes of a file into a buffer + * + * @src: file structure pointing to the file + * @max_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 +virStorageFileReadHeader(virStorageSourcePtr src, + ssize_t max_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->storageFileReadHeader) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("storage file header reading is not supported for " + "storage type %s (protocol: %s)"), + virStorageTypeToString(src->type), + virStorageNetProtocolTypeToString(src->protocol)); + return -2; + } + + ret = src->drv->backend->storageFileReadHeader(src, max_len, buf); + + VIR_DEBUG("read of storage header %p: ret=%zd", src, ret); + + 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); +} + + +/* 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) +{ + 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) { + if (src == parent) { + virReportSystemError(errno, + _("Cannot access storage file '%s' " + "(as uid:%u, gid:%u)"), + src->path, (unsigned int)uid, + (unsigned int)gid); + } else { + virReportSystemError(errno, + _("Cannot access backing file '%s' " + "of storage file '%s' (as uid:%u, gid:%u)"), + src->path, parent->path, + (unsigned int)uid, (unsigned int)gid); + } + + 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 = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER, + &buf)) < 0) + goto cleanup; + + if (virStorageFileGetMetadataInternal(src, buf, headerLen, + &backingFormat) < 0) + goto cleanup; + + /* check whether we need to go deeper */ + if (!src->backingStoreRaw) { + ret = 0; + goto cleanup; + } + + 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)) < 0) { + if (report_broken) + goto cleanup; + + /* if we fail somewhere midway, just accept and return a + * broken chain */ + ret = 0; + goto cleanup; + } + + src->backingStore = backingStore; + backingStore = NULL; + ret = 0; + + cleanup: + 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; + int ret = -1; + + if (!(cycle = virHashCreate(5, NULL))) + return -1; + + if (src->format <= VIR_STORAGE_FILE_NONE) + src->format = allow_probe ? + VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; + + ret = virStorageFileGetMetadataRecurse(src, src, uid, gid, + allow_probe, report_broken, cycle); + + 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 = virStorageFileReadHeader(src, 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 new file mode 100644 index 000000000..6b3362244 --- /dev/null +++ b/src/storage/storage_source.h @@ -0,0 +1,53 @@ +/* + * 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 virStorageFileReadHeader(virStorageSourcePtr src, + ssize_t max_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); + +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); + +#endif /* __VIR_STORAGE_SOURCE_H__ */ diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c index a73c53839..d630a213f 100644 --- a/tests/virstoragetest.c +++ b/tests/virstoragetest.c @@ -32,6 +32,7 @@ #include "dirname.h" #include "storage/storage_driver.h" +#include "storage/storage_source.h" #define VIR_FROM_THIS VIR_FROM_NONE -- 2.12.2

On 06/23/2017 09:33 AM, Peter Krempa wrote: [...] Should : ssize_t len = VIR_STORAGE_MAX_HEADER; change to size_t and a new variable "ssize_t read_len" be created and used for the following and subsequent virStorageFileGetMetadataFromBuf call? (although that also takes a size_t for the 3rd param). Probably should put the description below the args and restate since it reading @len bytes of data starting at @offset "read %zd bytes from storage %p starting at offset %zu", ret, src, offset With a few tweaks: Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John [...]

Refactor the access to storage driver usage along with qemuDomainStorageFileInit which ensures that we access the file with correct DAC uid/gid. --- src/qemu/qemu_driver.c | 25 ++++++++----------------- 1 file changed, 8 insertions(+), 17 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 4c4a6a036..115368d1d 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11309,9 +11309,9 @@ qemuDomainBlockPeek(virDomainPtr dom, unsigned int flags) { virQEMUDriverPtr driver = dom->conn->privateData; + virDomainDiskDefPtr disk = NULL; virDomainObjPtr vm; - int fd = -1, ret = -1; - const char *actual; + int ret = -1; virCheckFlags(0, -1); @@ -11322,32 +11322,23 @@ qemuDomainBlockPeek(virDomainPtr dom, goto cleanup; /* Check the path belongs to this domain. */ - if (!(actual = virDomainDiskPathByName(vm->def, path))) { + if (!(disk = virDomainDiskByName(vm->def, path, true))) { virReportError(VIR_ERR_INVALID_ARG, - _("invalid path '%s'"), path); + _("invalid disk or path '%s'"), path); goto cleanup; } - path = actual; - fd = qemuOpenFile(driver, vm, path, O_RDONLY, NULL, NULL); - if (fd < 0) + if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0) goto cleanup; - /* Seek and read. */ - /* NB. Because we configure with AC_SYS_LARGEFILE, off_t should - * be 64 bits on all platforms. - */ - if (lseek(fd, offset, SEEK_SET) == (off_t) -1 || - saferead(fd, buffer, size) == (ssize_t) -1) { - virReportSystemError(errno, - _("%s: failed to seek or read"), path); + if (virStorageFileRead(disk->src, offset, size, buffer) < 0) goto cleanup; - } ret = 0; cleanup: - VIR_FORCE_CLOSE(fd); + if (disk) + virStorageFileDeinit(disk->src); virDomainObjEndAPI(&vm); return ret; } -- 2.12.2

On 06/23/2017 09:33 AM, Peter Krempa wrote: Just noting that cleanup will call virStorageFileDeinit. Although virStorageFileInitAs wouldn't be called since tis won't call qemuDomainStorageFileInit, but it shouldn't matter since virStorageFileDeinit does check if virStorageFileIsInitialized - just a concern that some how some "future" change to Deinit does something that assumes Init was run. I would hope not, but just pointing it out... Reviewed-by: John Ferlan <jferlan(a)redhat.com&gt; John