8:33 a.m.
A collection of few cleanups and bugfixes.
Peter Krempa (6):
lib: Remove misplaced and redundant comments
tests: storage: Fully register storage driver
storage: Split out virStorageSource accessors to separate file
storage: Make virStorageFileReadHeader more universal
qemu: Use storage driver APIs in qemuDomainBlockPeek
qemu: Support only raw volumes in qemuDomainBlockPeek
po/POTFILES.in | 1 +
src/Makefile.am | 1 +
src/libvirt-domain.c | 4 +-
src/qemu/qemu_domain.c | 1 +
src/qemu/qemu_driver.c | 35 +-
src/security/virt-aa-helper.c | 2 +-
src/storage/storage_backend.h | 9 +-
src/storage/storage_backend_fs.c | 20 +-
src/storage/storage_backend_gluster.c | 37 ++-
src/storage/storage_driver.c | 551 +------------------------------
src/storage/storage_driver.h | 28 --
src/storage/storage_source.c | 587 ++++++++++++++++++++++++++++++++++
src/storage/storage_source.h | 54 ++++
tests/virstoragetest.c | 6 +-
14 files changed, 709 insertions(+), 627 deletions(-)
create mode 100644 src/storage/storage_source.c
create mode 100644 src/storage/storage_source.h
--
2.12.2
8:33 a.m.
New subject: [libvirt] [PATCH 1/6] lib: Remove misplaced and redundant comments
It's obvious that unsigned long long is 64 bit and also our web page
generator would misplace the comment after the return value due to the
way it's parsing them.
---
src/libvirt-domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/libvirt-domain.c b/src/libvirt-domain.c
index 9bda3c204..4033ae8e6 100644
--- a/src/libvirt-domain.c
+++ b/src/libvirt-domain.c
@@ -5820,7 +5820,7 @@ virDomainMemoryStats(virDomainPtr dom, virDomainMemoryStatPtr
stats,
int
virDomainBlockPeek(virDomainPtr dom,
const char *disk,
- unsigned long long offset /* really 64 bits */,
+ unsigned long long offset,
size_t size,
void *buffer,
unsigned int flags)
@@ -5956,7 +5956,7 @@ virDomainBlockResize(virDomainPtr dom,
*/
int
virDomainMemoryPeek(virDomainPtr dom,
- unsigned long long start /* really 64 bits */,
+ unsigned long long start,
size_t size,
void *buffer,
unsigned int flags)
--
2.12.2
9:41 a.m.
New subject: [libvirt] [PATCH 1/6] lib: Remove misplaced and redundant comments
On 06/23/2017 09:33 AM, Peter Krempa wrote:
It's obvious that unsigned long long is 64 bit and also our web
page
generator would misplace the comment after the return value due to the
way it's parsing them.
---
src/libvirt-domain.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
(this one's safe for freeze if you care, but the rest should wait for
after release)
5:20 a.m.
New subject: [libvirt] [PATCH 1/6] lib: Remove misplaced and redundant comments
On Sat, Jul 01, 2017 at 10:41:24 -0400, John Ferlan wrote:
On 06/23/2017 09:33 AM, Peter Krempa wrote:
> It's obvious that unsigned long long is 64 bit and also our web page
> generator would misplace the comment after the return value due to the
> way it's parsing them.
> ---
> src/libvirt-domain.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
(this one's safe for freeze if you care, but the rest should wait for
after release)
I've pushed this one so that the docs embedded in the upcomming release
are already fixed.
Thanks.
8:33 a.m.
New subject: [libvirt] [PATCH 2/6] tests: storage: Fully register storage driver
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
9:41 a.m.
New subject: [libvirt] [PATCH 2/6] tests: storage: Fully register storage driver
On 06/23/2017 09:33 AM, Peter Krempa wrote:
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(-)
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
8:33 a.m.
New subject: [libvirt] [PATCH 3/6] storage: Split out virStorageSource accessors to separate file
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
9:42 a.m.
New subject: [libvirt] [PATCH 3/6] storage: Split out virStorageSource accessors to separate file
On 06/23/2017 09:33 AM, Peter Krempa wrote:
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
Since all of the helpers being moved are prefixed with virStorageFile
why not "storage_file.{c,h}"? I realize the helpers are all operating on
virStorageSourcePtr. Is it perhaps because being too close to
virstoragefile.{c,h}?
It's not that important, but I suppose I'd expect virStorageSource
helper prefixes in a storage_source.c.
If you do decide to change the name, be sure to adjust the VIR_LOG_INIT
appropriately too. And you could have also adjusted the .h prototypes to
follow the more recently decided upon format that follows the .c files.
As an aside - the remaining functions at the bottom of storage_driver
probably could move to storage_util. Yeah, I know - patches are welcome.
I'd prefer to see storage_file, but again it's not that important...
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
9:46 a.m.
New subject: [libvirt] [PATCH 3/6] storage: Split out virStorageSource accessors to separate file
On Sat, Jul 01, 2017 at 10:42:56 -0400, John Ferlan wrote:
On 06/23/2017 09:33 AM, Peter Krempa wrote:
> 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
>
Since all of the helpers being moved are prefixed with virStorageFile
why not "storage_file.{c,h}"? I realize the helpers are all operating on
virStorageSourcePtr. Is it perhaps because being too close to
virstoragefile.{c,h}?
The thing is that the virStorageSource does not necessarily need to be a
file so I'd eventually like to rename those helpers, so I'll stick with
this filename for now.
8:33 a.m.
New subject: [libvirt] [PATCH 4/6] storage: Make virStorageFileReadHeader more universal
Allow specifying offset to read an arbitrary position in the file. This
warrants a rename to virStorageFileRead.
---
src/qemu/qemu_driver.c | 3 +--
src/storage/storage_backend.h | 9 +++++----
src/storage/storage_backend_fs.c | 20 +++++++++++++------
src/storage/storage_backend_gluster.c | 37 ++++++++++++++++++++++-------------
src/storage/storage_source.c | 30 +++++++++++++++-------------
src/storage/storage_source.h | 7 ++++---
6 files changed, 63 insertions(+), 43 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 052c7f0fc..4c4a6a036 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11584,8 +11584,7 @@ qemuStorageLimitsRefresh(virQEMUDriverPtr driver,
goto cleanup;
}
} else {
- if ((len = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
- &buf)) < 0)
+ if ((len = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER, &buf)) < 0)
goto cleanup;
}
diff --git a/src/storage/storage_backend.h b/src/storage/storage_backend.h
index 21ac84552..193cf134d 100644
--- a/src/storage/storage_backend.h
+++ b/src/storage/storage_backend.h
@@ -154,9 +154,10 @@ typedef int
struct stat *st);
typedef ssize_t
-(*virStorageFileBackendReadHeader)(virStorageSourcePtr src,
- ssize_t max_len,
- char **buf);
+(*virStorageFileBackendRead)(virStorageSourcePtr src,
+ size_t offset,
+ size_t len,
+ char **buf);
typedef const char *
(*virStorageFileBackendGetUniqueIdentifier)(virStorageSourcePtr src);
@@ -186,7 +187,7 @@ struct _virStorageFileBackend {
* error on failure. */
virStorageFileBackendInit backendInit;
virStorageFileBackendDeinit backendDeinit;
- virStorageFileBackendReadHeader storageFileReadHeader;
+ virStorageFileBackendRead storageFileRead;
virStorageFileBackendGetUniqueIdentifier storageFileGetUniqueIdentifier;
/* The following group of callbacks is expected to set errno
diff --git a/src/storage/storage_backend_fs.c b/src/storage/storage_backend_fs.c
index bf1d7de43..847dbc9e0 100644
--- a/src/storage/storage_backend_fs.c
+++ b/src/storage/storage_backend_fs.c
@@ -779,9 +779,10 @@ virStorageFileBackendFileStat(virStorageSourcePtr src,
static ssize_t
-virStorageFileBackendFileReadHeader(virStorageSourcePtr src,
- ssize_t max_len,
- char **buf)
+virStorageFileBackendFileRead(virStorageSourcePtr src,
+ size_t offset,
+ size_t len,
+ char **buf)
{
int fd = -1;
ssize_t ret = -1;
@@ -793,7 +794,14 @@ virStorageFileBackendFileReadHeader(virStorageSourcePtr src,
return -1;
}
- if ((ret = virFileReadHeaderFD(fd, max_len, buf)) < 0) {
+ if (offset > 0) {
+ if (lseek(fd, offset, SEEK_SET) == (off_t) -1) {
+ virReportSystemError(errno, _("cannot seek into '%s'"),
src->path);
+ goto cleanup;
+ }
+ }
+
+ if ((ret = virFileReadHeaderFD(fd, len, buf)) < 0) {
virReportSystemError(errno,
_("cannot read header '%s'"),
src->path);
goto cleanup;
@@ -850,7 +858,7 @@ virStorageFileBackend virStorageFileBackendFile = {
.storageFileCreate = virStorageFileBackendFileCreate,
.storageFileUnlink = virStorageFileBackendFileUnlink,
.storageFileStat = virStorageFileBackendFileStat,
- .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+ .storageFileRead = virStorageFileBackendFileRead,
.storageFileAccess = virStorageFileBackendFileAccess,
.storageFileChown = virStorageFileBackendFileChown,
@@ -865,7 +873,7 @@ virStorageFileBackend virStorageFileBackendBlock = {
.backendDeinit = virStorageFileBackendFileDeinit,
.storageFileStat = virStorageFileBackendFileStat,
- .storageFileReadHeader = virStorageFileBackendFileReadHeader,
+ .storageFileRead = virStorageFileBackendFileRead,
.storageFileAccess = virStorageFileBackendFileAccess,
.storageFileChown = virStorageFileBackendFileChown,
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 93dce4042..8ea7e603c 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -151,20 +151,20 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
static ssize_t
-virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
- const char *name,
- ssize_t maxlen,
- char **buf)
+virStorageBackendGlusterRead(glfs_fd_t *fd,
+ const char *name,
+ size_t len,
+ char **buf)
{
char *s;
size_t nread = 0;
- if (VIR_ALLOC_N(*buf, maxlen) < 0)
+ if (VIR_ALLOC_N(*buf, len) < 0)
return -1;
s = *buf;
- while (maxlen) {
- ssize_t r = glfs_read(fd, s, maxlen, 0);
+ while (len) {
+ ssize_t r = glfs_read(fd, s, len, 0);
if (r < 0 && errno == EINTR)
continue;
if (r < 0) {
@@ -175,7 +175,7 @@ virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
if (r == 0)
return nread;
s += r;
- maxlen -= r;
+ len -= r;
nread += r;
}
return nread;
@@ -292,7 +292,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr
state,
goto cleanup;
}
- if ((len = virStorageBackendGlusterReadHeader(fd, name, len, &header)) < 0)
+ if ((len = virStorageBackendGlusterRead(fd, name, len, &header)) < 0)
goto cleanup;
if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len,
@@ -721,9 +721,10 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
static ssize_t
-virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
- ssize_t max_len,
- char **buf)
+virStorageFileBackendGlusterRead(virStorageSourcePtr src,
+ size_t offset,
+ size_t len,
+ char **buf)
{
virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
glfs_fd_t *fd = NULL;
@@ -737,8 +738,16 @@ virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
return -1;
}
- ret = virStorageBackendGlusterReadHeader(fd, src->path, max_len, buf);
+ if (offset > 0) {
+ if (glfs_lseek(fd, offset, SEEK_SET) == (off_t) -1) {
+ virReportSystemError(errno, _("cannot seek into '%s'"),
src->path);
+ goto cleanup;
+ }
+ }
+
+ ret = virStorageBackendGlusterRead(fd, src->path, len, buf);
+ cleanup:
if (fd)
glfs_close(fd);
@@ -851,7 +860,7 @@ virStorageFileBackend virStorageFileBackendGluster = {
.storageFileCreate = virStorageFileBackendGlusterCreate,
.storageFileUnlink = virStorageFileBackendGlusterUnlink,
.storageFileStat = virStorageFileBackendGlusterStat,
- .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
+ .storageFileRead = virStorageFileBackendGlusterRead,
.storageFileAccess = virStorageFileBackendGlusterAccess,
.storageFileChown = virStorageFileBackendGlusterChown,
diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index c91d629c8..130c2d2f3 100644
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -64,7 +64,7 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
}
return backend->storageFileGetUniqueIdentifier &&
- backend->storageFileReadHeader &&
+ backend->storageFileRead &&
backend->storageFileAccess;
}
@@ -263,10 +263,11 @@ virStorageFileStat(virStorageSourcePtr src,
/**
- * virStorageFileReadHeader: read the beginning bytes of a file into a buffer
+ * virStorageFileRead: 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
+ * @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
@@ -274,9 +275,10 @@ virStorageFileStat(virStorageSourcePtr src,
* Libvirt error is reported on failure.
*/
ssize_t
-virStorageFileReadHeader(virStorageSourcePtr src,
- ssize_t max_len,
- char **buf)
+virStorageFileRead(virStorageSourcePtr src,
+ size_t offset,
+ size_t len,
+ char **buf)
{
ssize_t ret;
@@ -286,18 +288,18 @@ virStorageFileReadHeader(virStorageSourcePtr src,
return -1;
}
- if (!src->drv->backend->storageFileReadHeader) {
+ if (!src->drv->backend->storageFileRead) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("storage file header reading is not supported for "
+ _("storage file 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);
+ ret = src->drv->backend->storageFileRead(src, offset, len, buf);
- VIR_DEBUG("read of storage header %p: ret=%zd", src, ret);
+ VIR_DEBUG("read of storage %p: ret=%zd", src, ret);
return ret;
}
@@ -444,8 +446,8 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
if (virHashAddEntry(cycle, uniqueName, (void *)1) < 0)
goto cleanup;
- if ((headerLen = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
- &buf)) < 0)
+ if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER,
+ &buf)) < 0)
goto cleanup;
if (virStorageFileGetMetadataInternal(src, buf, headerLen,
@@ -565,8 +567,8 @@ virStorageFileGetBackingStoreStr(virStorageSourcePtr src)
if (virStorageFileAccess(src, F_OK) < 0)
return NULL;
- if ((headerLen = virStorageFileReadHeader(src, VIR_STORAGE_MAX_HEADER,
- &buf)) < 0)
+ if ((headerLen = virStorageFileRead(src, 0, VIR_STORAGE_MAX_HEADER,
+ &buf)) < 0)
return NULL;
if (!(tmp = virStorageSourceCopy(src, false)))
diff --git a/src/storage/storage_source.h b/src/storage/storage_source.h
index 6b3362244..6462baf6a 100644
--- a/src/storage/storage_source.h
+++ b/src/storage/storage_source.h
@@ -32,9 +32,10 @@ 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);
+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);
--
2.12.2
9:43 a.m.
New subject: [libvirt] [PATCH 4/6] storage: Make virStorageFileReadHeader more universal
On 06/23/2017 09:33 AM, Peter Krempa wrote:
Allow specifying offset to read an arbitrary position in the file.
This
warrants a rename to virStorageFileRead.
---
src/qemu/qemu_driver.c | 3 +--
src/storage/storage_backend.h | 9 +++++----
src/storage/storage_backend_fs.c | 20 +++++++++++++------
src/storage/storage_backend_gluster.c | 37 ++++++++++++++++++++++-------------
src/storage/storage_source.c | 30 +++++++++++++++-------------
src/storage/storage_source.h | 7 ++++---
6 files changed, 63 insertions(+), 43 deletions(-)
[...]
diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
index 93dce4042..8ea7e603c 100644
--- a/src/storage/storage_backend_gluster.c
+++ b/src/storage/storage_backend_gluster.c
@@ -151,20 +151,20 @@ virStorageBackendGlusterOpen(virStoragePoolObjPtr pool)
static ssize_t
-virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
- const char *name,
- ssize_t maxlen,
- char **buf)
+virStorageBackendGlusterRead(glfs_fd_t *fd,
+ const char *name,
+ size_t len,
+ char **buf)
{
char *s;
size_t nread = 0;
- if (VIR_ALLOC_N(*buf, maxlen) < 0)
+ if (VIR_ALLOC_N(*buf, len) < 0)
return -1;
s = *buf;
- while (maxlen) {
- ssize_t r = glfs_read(fd, s, maxlen, 0);
+ while (len) {
+ ssize_t r = glfs_read(fd, s, len, 0);
if (r < 0 && errno == EINTR)
continue;
if (r < 0) {
@@ -175,7 +175,7 @@ virStorageBackendGlusterReadHeader(glfs_fd_t *fd,
if (r == 0)
return nread;
s += r;
- maxlen -= r;
+ len -= r;
nread += r;
}
return nread;
@@ -292,7 +292,7 @@ virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr
state,
goto cleanup;
}
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).
- if ((len = virStorageBackendGlusterReadHeader(fd, name, len,
&header)) < 0)
+ if ((len = virStorageBackendGlusterRead(fd, name, len, &header)) < 0)
goto cleanup;
if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len,
@@ -721,9 +721,10 @@ virStorageFileBackendGlusterStat(virStorageSourcePtr src,
static ssize_t
-virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
- ssize_t max_len,
- char **buf)
+virStorageFileBackendGlusterRead(virStorageSourcePtr src,
+ size_t offset,
+ size_t len,
+ char **buf)
{
virStorageFileBackendGlusterPrivPtr priv = src->drv->priv;
glfs_fd_t *fd = NULL;
@@ -737,8 +738,16 @@ virStorageFileBackendGlusterReadHeader(virStorageSourcePtr src,
return -1;
}
- ret = virStorageBackendGlusterReadHeader(fd, src->path, max_len, buf);
+ if (offset > 0) {
+ if (glfs_lseek(fd, offset, SEEK_SET) == (off_t) -1) {
+ virReportSystemError(errno, _("cannot seek into '%s'"),
src->path);
+ goto cleanup;
+ }
+ }
+
+ ret = virStorageBackendGlusterRead(fd, src->path, len, buf);
+ cleanup:
if (fd)
glfs_close(fd);
@@ -851,7 +860,7 @@ virStorageFileBackend virStorageFileBackendGluster = {
.storageFileCreate = virStorageFileBackendGlusterCreate,
.storageFileUnlink = virStorageFileBackendGlusterUnlink,
.storageFileStat = virStorageFileBackendGlusterStat,
- .storageFileReadHeader = virStorageFileBackendGlusterReadHeader,
+ .storageFileRead = virStorageFileBackendGlusterRead,
.storageFileAccess = virStorageFileBackendGlusterAccess,
.storageFileChown = virStorageFileBackendGlusterChown,
diff --git a/src/storage/storage_source.c b/src/storage/storage_source.c
index c91d629c8..130c2d2f3 100644
--- a/src/storage/storage_source.c
+++ b/src/storage/storage_source.c
@@ -64,7 +64,7 @@ virStorageFileSupportsBackingChainTraversal(virStorageSourcePtr src)
}
return backend->storageFileGetUniqueIdentifier &&
- backend->storageFileReadHeader &&
+ backend->storageFileRead &&
backend->storageFileAccess;
}
@@ -263,10 +263,11 @@ virStorageFileStat(virStorageSourcePtr src,
/**
- * virStorageFileReadHeader: read the beginning bytes of a file into a buffer
+ * virStorageFileRead: read the beginning bytes of a file into a buffer
Probably should put the description below the args and restate since it
reading @len bytes of data starting at @offset
*
* @src: file structure pointing to the file
- * @max_len: maximum number of bytes read from the storage 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
@@ -274,9 +275,10 @@ virStorageFileStat(virStorageSourcePtr src,
* Libvirt error is reported on failure.
*/
ssize_t
-virStorageFileReadHeader(virStorageSourcePtr src,
- ssize_t max_len,
- char **buf)
+virStorageFileRead(virStorageSourcePtr src,
+ size_t offset,
+ size_t len,
+ char **buf)
{
ssize_t ret;
@@ -286,18 +288,18 @@ virStorageFileReadHeader(virStorageSourcePtr src,
return -1;
}
- if (!src->drv->backend->storageFileReadHeader) {
+ if (!src->drv->backend->storageFileRead) {
virReportError(VIR_ERR_INTERNAL_ERROR,
- _("storage file header reading is not supported for "
+ _("storage file 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);
+ ret = src->drv->backend->storageFileRead(src, offset, len, buf);
- VIR_DEBUG("read of storage header %p: ret=%zd", src, ret);
+ VIR_DEBUG("read of storage %p: ret=%zd", src, ret);
"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>
John
[...]
10:04 a.m.
New subject: [libvirt] [PATCH 4/6] storage: Make virStorageFileReadHeader more universal
On Sat, Jul 01, 2017 at 10:43:09 -0400, John Ferlan wrote:
On 06/23/2017 09:33 AM, Peter Krempa wrote:
> Allow specifying offset to read an arbitrary position in the file. This
> warrants a rename to virStorageFileRead.
> ---
> src/qemu/qemu_driver.c | 3 +--
> src/storage/storage_backend.h | 9 +++++----
> src/storage/storage_backend_fs.c | 20 +++++++++++++------
> src/storage/storage_backend_gluster.c | 37 ++++++++++++++++++++++-------------
> src/storage/storage_source.c | 30 +++++++++++++++-------------
> src/storage/storage_source.h | 7 ++++---
> 6 files changed, 63 insertions(+), 43 deletions(-)
>
[...]
> diff --git a/src/storage/storage_backend_gluster.c
b/src/storage/storage_backend_gluster.c
> index 93dce4042..8ea7e603c 100644
> --- a/src/storage/storage_backend_gluster.c
> +++ b/src/storage/storage_backend_gluster.c
[...]
> @@ -292,7 +292,7 @@
virStorageBackendGlusterRefreshVol(virStorageBackendGlusterStatePtr state,
> goto cleanup;
> }
>
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).
I actually just used VIR_STORAGE_MAX_HEADER constant directly in the
call of virStorageBackendGlusterRead. The original variable will thus be
used only for the return value, which is signed.
> - if ((len = virStorageBackendGlusterReadHeader(fd, name, len, &header)) <
0)
> + if ((len = virStorageBackendGlusterRead(fd, name, len, &header)) < 0)
> goto cleanup;
>
> if (!(meta = virStorageFileGetMetadataFromBuf(name, header, len,
8:33 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: Use storage driver APIs in qemuDomainBlockPeek
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
9:43 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: Use storage driver APIs in qemuDomainBlockPeek
On 06/23/2017 09:33 AM, Peter Krempa wrote:
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(-)
Irony is the comment at the end of qemuOpenFile:
* This function should not be used on storage sources. Use
* qemuDomainStorageFileInit and storage driver APIs if possible.
Reviewed-by: John Ferlan <jferlan(a)redhat.com>
John
8:10 a.m.
New subject: [libvirt] [PATCH 5/6] qemu: Use storage driver APIs in qemuDomainBlockPeek
On Sat, Jul 01, 2017 at 10:43:23 -0400, John Ferlan wrote:
On 06/23/2017 09:33 AM, Peter Krempa wrote:
> 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(-)
>
Irony is the comment at the end of qemuOpenFile:
* This function should not be used on storage sources. Use
* qemuDomainStorageFileInit and storage driver APIs if possible.
This message was added by:
commit f7105d0e4a485bf7d9e878fd17e675d7f9d29f9f
Author: Peter Krempa <pkrempa(a)redhat.com>
Date: Wed May 10 12:28:38 2017 +0200
the usage of qemuOpenFile (prior to that, open() was used directly) was
added in:
commit b4a40dd92dc7e6f110b13f2353cb5343d1147227
Author: Martin Kletzander <mkletzan(a)redhat.com>
Date: Fri May 24 18:26:26 2013 +0200
I was just lazy and did not bother fixing all places right away and
added the warning that it shouldn't be reused more.
8:33 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: Support only raw volumes in qemuDomainBlockPeek
The API documents that it peeks into the VM disk. We can't do that
currently for non raw images so report an error.
---
src/qemu/qemu_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 115368d1d..53655749b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11328,6 +11328,12 @@ qemuDomainBlockPeek(virDomainPtr dom,
goto cleanup;
}
+ if (disk->src->format != VIR_STORAGE_FILE_RAW) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("peeking is supported only for RAW disks"));
+ goto cleanup;
+ }
+
if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0)
goto cleanup;
--
2.12.2
9:45 a.m.
New subject: [libvirt] [PATCH 6/6] qemu: Support only raw volumes in qemuDomainBlockPeek
On 06/23/2017 09:33 AM, Peter Krempa wrote:
The API documents that it peeks into the VM disk. We can't do
that
currently for non raw images so report an error.
---
src/qemu/qemu_driver.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 115368d1d..53655749b 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -11328,6 +11328,12 @@ qemuDomainBlockPeek(virDomainPtr dom,
goto cleanup;
}
+ if (disk->src->format != VIR_STORAGE_FILE_RAW) {
+ virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+ _("peeking is supported only for RAW disks"));
+ goto cleanup;
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>
John
+ }
+
if (qemuDomainStorageFileInit(driver, vm, disk->src) < 0)
goto cleanup;
2693
days inactive
2711
days old
16 comments
2 participants
participants (2)
-
John Ferlan -
Peter Krempa