[libvirt] [PATCHv2 11/33] storage: Move virStorageFileGetMetadata to the storage driver
Peter Krempa
pkrempa at redhat.com
Thu May 22 13:47:50 UTC 2014
My future work will modify the metadata crawler function to use the
storage driver file APIs to access the files instead of accessing them
directly so that we will be able to request the metadata for remote
files too. To avoid linking the storage driver to every helper file
using the utils code, the backing chain traversal function needs to be
moved to the storage driver source.
Additionally the virt-aa-helper and virstoragetest programs need to be
linked with the storage driver as a result of this change.
---
cfg.mk | 2 +-
src/Makefile.am | 2 +
src/libvirt_private.syms | 2 +-
src/qemu/qemu_domain.c | 2 +
src/security/virt-aa-helper.c | 2 +
src/storage/storage_driver.c | 233 ++++++++++++++++++++++++++++++++++++++++++
src/storage/storage_driver.h | 5 +
src/util/virstoragefile.c | 233 +-----------------------------------------
src/util/virstoragefile.h | 7 +-
tests/Makefile.am | 7 +-
tests/virstoragetest.c | 2 +
11 files changed, 258 insertions(+), 239 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index 5ff2721..9e8fcec 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -774,7 +774,7 @@ sc_prohibit_cross_inclusion:
access/ | conf/) safe="($$dir|conf|util)";; \
locking/) safe="($$dir|util|conf|rpc)";; \
cpu/| network/| node_device/| rpc/| security/| storage/) \
- safe="($$dir|util|conf)";; \
+ safe="($$dir|util|conf|storage)";; \
xenapi/ | xenxs/ ) safe="($$dir|util|conf|xen)";; \
*) safe="($$dir|$(mid_dirs)|util)";; \
esac; \
diff --git a/src/Makefile.am b/src/Makefile.am
index cfb7097..5028f0d 100644
--- a/src/Makefile.am
+++ b/src/Makefile.am
@@ -2565,8 +2565,10 @@ virt_aa_helper_LDFLAGS = \
$(PIE_LDFLAGS) \
$(NULL)
virt_aa_helper_LDADD = \
+ libvirt.la \
libvirt_conf.la \
libvirt_util.la \
+ libvirt_driver_storage_impl.la \
../gnulib/lib/libgnu.la
if WITH_DTRACE_PROBES
virt_aa_helper_LDADD += libvirt_probes.lo
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index c3332c9..25dab2d 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1863,9 +1863,9 @@ virStorageFileFeatureTypeToString;
virStorageFileFormatTypeFromString;
virStorageFileFormatTypeToString;
virStorageFileGetLVMKey;
-virStorageFileGetMetadata;
virStorageFileGetMetadataFromBuf;
virStorageFileGetMetadataFromFD;
+virStorageFileGetMetadataFromFDInternal;
virStorageFileGetSCSIKey;
virStorageFileIsClusterFS;
virStorageFileParseChainIndex;
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 78cfdc6..502b7bd 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -40,6 +40,8 @@
#include "virstoragefile.h"
#include "virstring.h"
+#include "storage/storage_driver.h"
+
#include <sys/time.h>
#include <fcntl.h>
diff --git a/src/security/virt-aa-helper.c b/src/security/virt-aa-helper.c
index 32fc04a..bf540b4 100644
--- a/src/security/virt-aa-helper.c
+++ b/src/security/virt-aa-helper.c
@@ -55,6 +55,8 @@
#include "virrandom.h"
#include "virstring.h"
+#include "storage/storage_driver.h"
+
#define VIR_FROM_THIS VIR_FROM_SECURITY
static char *progname;
diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index cb660ea..c90107b 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -49,6 +49,7 @@
#include "configmake.h"
#include "virstring.h"
#include "viraccessapicheck.h"
+#include "dirname.h"
#define VIR_FROM_THIS VIR_FROM_STORAGE
@@ -3041,3 +3042,235 @@ virStorageFileAccess(virStorageSourcePtr src,
return src->drv->backend->storageFileAccess(src, mode);
}
+
+
+/**
+ * Given a starting point START (a directory containing the original
+ * file, if the original file was opened via a relative path; ignored
+ * if NAME is absolute), determine the location of the backing file
+ * NAME (possibly relative), and compute the relative DIRECTORY
+ * (optional) and CANONICAL (mandatory) location of the backing file.
+ * Return 0 on success, negative on error.
+ */
+static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
+virFindBackingFile(const char *start, const char *path,
+ char **directory, char **canonical)
+{
+ /* FIXME - when we eventually allow non-raw network devices, we
+ * must ensure that we handle backing files the same way as qemu.
+ * For a qcow2 top file of gluster://server/vol/img, qemu treats
+ * the relative backing file 'rel' as meaning
+ * 'gluster://server/vol/rel', while the backing file '/abs' is
+ * used as a local file. But we cannot canonicalize network
+ * devices via canonicalize_file_name(), because they are not part
+ * of the local file system. */
+ char *combined = NULL;
+ int ret = -1;
+
+ if (*path == '/') {
+ /* Safe to cast away const */
+ combined = (char *)path;
+ } else if (virAsprintf(&combined, "%s/%s", start, path) < 0) {
+ goto cleanup;
+ }
+
+ if (directory && !(*directory = mdir_name(combined))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+
+ if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid()) < 0) {
+ virReportSystemError(errno,
+ _("Cannot access backing file '%s'"),
+ combined);
+ goto cleanup;
+ }
+
+ if (!(*canonical = canonicalize_file_name(combined))) {
+ virReportSystemError(errno,
+ _("Can't canonicalize path '%s'"), path);
+ goto cleanup;
+ }
+
+ ret = 0;
+
+ cleanup:
+ if (combined != path)
+ VIR_FREE(combined);
+ return ret;
+}
+
+
+/* Recursive workhorse for virStorageFileGetMetadata. */
+static int
+virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
+ const char *canonPath,
+ uid_t uid, gid_t gid,
+ bool allow_probe,
+ virHashTablePtr cycle)
+{
+ int fd;
+ int ret = -1;
+ virStorageSourcePtr backingStore = NULL;
+ int backingFormat;
+
+ VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d",
+ src->path, canonPath, NULLSTR(src->relDir), src->format,
+ (int)uid, (int)gid, allow_probe);
+
+ if (virHashLookup(cycle, canonPath)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("backing store for %s is self-referential"),
+ src->path);
+ return -1;
+ }
+
+ if (virHashAddEntry(cycle, canonPath, (void *)1) < 0)
+ return -1;
+
+ if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
+ if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) {
+ virReportSystemError(-fd, _("Failed to open file '%s'"),
+ src->path);
+ return -1;
+ }
+
+ if (virStorageFileGetMetadataFromFDInternal(src, fd,
+ &backingFormat) < 0) {
+ VIR_FORCE_CLOSE(fd);
+ return -1;
+ }
+
+ if (VIR_CLOSE(fd) < 0)
+ VIR_WARN("could not close file %s", src->path);
+ } else {
+ /* TODO: currently we call this only for local storage */
+ return 0;
+ }
+
+ /* check whether we need to go deeper */
+ if (!src->backingStoreRaw)
+ return 0;
+
+ if (VIR_ALLOC(backingStore) < 0)
+ return -1;
+
+ if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0)
+ goto cleanup;
+
+ if (virStorageIsFile(src->backingStoreRaw)) {
+ backingStore->type = VIR_STORAGE_TYPE_FILE;
+
+ if (virFindBackingFile(src->relDir,
+ src->backingStoreRaw,
+ &backingStore->relDir,
+ &backingStore->path) < 0) {
+ /* the backing file is (currently) unavailable, treat this
+ * file as standalone:
+ * backingStoreRaw is kept to mark broken image chains */
+ VIR_WARN("Backing file '%s' of image '%s' is missing.",
+ src->backingStoreRaw, src->path);
+ ret = 0;
+ goto cleanup;
+ }
+ } else {
+ /* TODO: To satisfy the test case, copy the network URI as path. This
+ * will be removed later. */
+ backingStore->type = VIR_STORAGE_TYPE_NETWORK;
+
+ if (VIR_STRDUP(backingStore->path, src->backingStoreRaw) < 0)
+ 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 (virStorageFileGetMetadataRecurse(backingStore,
+ backingStore->path,
+ uid, gid, allow_probe,
+ cycle) < 0) {
+ /* if we fail somewhere midway, just accept and return a
+ * broken chain */
+ ret = 0;
+ goto cleanup;
+ }
+
+ src->backingStore = backingStore;
+ backingStore = NULL;
+ ret = 0;
+
+ cleanup:
+ 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.
+ *
+ * Caller MUST free result after use via virStorageSourceFree.
+ */
+int
+virStorageFileGetMetadata(virStorageSourcePtr src,
+ uid_t uid, gid_t gid,
+ bool allow_probe)
+{
+ VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d",
+ src->path, src->format, (int)uid, (int)gid, allow_probe);
+
+ virHashTablePtr cycle = NULL;
+ char *canonPath = NULL;
+ int ret = -1;
+
+ if (!(cycle = virHashCreate(5, NULL)))
+ return -1;
+
+ if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
+ if (!(canonPath = canonicalize_file_name(src->path))) {
+ virReportSystemError(errno, _("unable to resolve '%s'"),
+ src->path);
+ goto cleanup;
+ }
+
+ if (!src->relPath &&
+ VIR_STRDUP(src->relPath, src->path) < 0)
+ goto cleanup;
+
+ if (!src->relDir &&
+ !(src->relDir = mdir_name(src->path))) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ } else {
+ /* TODO: currently unimplemented for non-local storage */
+ ret = 0;
+ goto cleanup;
+ }
+
+ if (src->format <= VIR_STORAGE_FILE_NONE)
+ src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
+
+ ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid,
+ allow_probe, cycle);
+
+ cleanup:
+ VIR_FREE(canonPath);
+ virHashFree(cycle);
+ return ret;
+}
diff --git a/src/storage/storage_driver.h b/src/storage/storage_driver.h
index 5452df2..bd9e9ed 100644
--- a/src/storage/storage_driver.h
+++ b/src/storage/storage_driver.h
@@ -44,6 +44,11 @@ ssize_t virStorageFileReadHeader(virStorageSourcePtr src,
const char *virStorageFileGetUniqueIdentifier(virStorageSourcePtr src);
int virStorageFileAccess(virStorageSourcePtr src, int mode);
+int virStorageFileGetMetadata(virStorageSourcePtr src,
+ uid_t uid, gid_t gid,
+ bool allow_probe)
+ ATTRIBUTE_NONNULL(1);
+
int storageRegister(void);
#endif /* __VIR_STORAGE_DRIVER_H__ */
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index b90cdc9..06bb68c 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -28,7 +28,6 @@
#include <unistd.h>
#include <fcntl.h>
#include <stdlib.h>
-#include "dirname.h"
#include "viralloc.h"
#include "virerror.h"
#include "virlog.h"
@@ -565,62 +564,6 @@ qedGetBackingStore(char **res,
return BACKING_STORE_OK;
}
-/**
- * Given a starting point START (a directory containing the original
- * file, if the original file was opened via a relative path; ignored
- * if NAME is absolute), determine the location of the backing file
- * NAME (possibly relative), and compute the relative DIRECTORY
- * (optional) and CANONICAL (mandatory) location of the backing file.
- * Return 0 on success, negative on error.
- */
-static int ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4)
-virFindBackingFile(const char *start, const char *path,
- char **directory, char **canonical)
-{
- /* FIXME - when we eventually allow non-raw network devices, we
- * must ensure that we handle backing files the same way as qemu.
- * For a qcow2 top file of gluster://server/vol/img, qemu treats
- * the relative backing file 'rel' as meaning
- * 'gluster://server/vol/rel', while the backing file '/abs' is
- * used as a local file. But we cannot canonicalize network
- * devices via canonicalize_file_name(), because they are not part
- * of the local file system. */
- char *combined = NULL;
- int ret = -1;
-
- if (*path == '/') {
- /* Safe to cast away const */
- combined = (char *)path;
- } else if (virAsprintf(&combined, "%s/%s", start, path) < 0) {
- goto cleanup;
- }
-
- if (directory && !(*directory = mdir_name(combined))) {
- virReportOOMError();
- goto cleanup;
- }
-
- if (virFileAccessibleAs(combined, F_OK, geteuid(), getegid()) < 0) {
- virReportSystemError(errno,
- _("Cannot access backing file '%s'"),
- combined);
- goto cleanup;
- }
-
- if (!(*canonical = canonicalize_file_name(combined))) {
- virReportSystemError(errno,
- _("Can't canonicalize path '%s'"), path);
- goto cleanup;
- }
-
- ret = 0;
-
- cleanup:
- if (combined != path)
- VIR_FREE(combined);
- return ret;
-}
-
static bool
virStorageFileMatchesMagic(int format,
@@ -1019,7 +962,7 @@ virStorageFileGetMetadataFromBuf(const char *path,
/* Internal version that also supports a containing directory name. */
-static int
+int
virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
int fd,
int *backingFormat)
@@ -1108,180 +1051,6 @@ virStorageFileGetMetadataFromFD(const char *path,
}
-/* Recursive workhorse for virStorageFileGetMetadata. */
-static int
-virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
- const char *canonPath,
- uid_t uid, gid_t gid,
- bool allow_probe,
- virHashTablePtr cycle)
-{
- int fd;
- int ret = -1;
- virStorageSourcePtr backingStore = NULL;
- int backingFormat;
-
- VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d",
- src->path, canonPath, NULLSTR(src->relDir), src->format,
- (int)uid, (int)gid, allow_probe);
-
- if (virHashLookup(cycle, canonPath)) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("backing store for %s is self-referential"),
- src->path);
- return -1;
- }
-
- if (virHashAddEntry(cycle, canonPath, (void *)1) < 0)
- return -1;
-
- if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
- if ((fd = virFileOpenAs(src->path, O_RDONLY, 0, uid, gid, 0)) < 0) {
- virReportSystemError(-fd, _("Failed to open file '%s'"),
- src->path);
- return -1;
- }
-
- if (virStorageFileGetMetadataFromFDInternal(src, fd,
- &backingFormat) < 0) {
- VIR_FORCE_CLOSE(fd);
- return -1;
- }
-
- if (VIR_CLOSE(fd) < 0)
- VIR_WARN("could not close file %s", src->path);
- } else {
- /* TODO: currently we call this only for local storage */
- return 0;
- }
-
- /* check whether we need to go deeper */
- if (!src->backingStoreRaw)
- return 0;
-
- if (VIR_ALLOC(backingStore) < 0)
- return -1;
-
- if (VIR_STRDUP(backingStore->relPath, src->backingStoreRaw) < 0)
- goto cleanup;
-
- if (virStorageIsFile(src->backingStoreRaw)) {
- backingStore->type = VIR_STORAGE_TYPE_FILE;
-
- if (virFindBackingFile(src->relDir,
- src->backingStoreRaw,
- &backingStore->relDir,
- &backingStore->path) < 0) {
- /* the backing file is (currently) unavailable, treat this
- * file as standalone:
- * backingStoreRaw is kept to mark broken image chains */
- VIR_WARN("Backing file '%s' of image '%s' is missing.",
- src->backingStoreRaw, src->path);
- ret = 0;
- goto cleanup;
- }
- } else {
- /* TODO: To satisfy the test case, copy the network URI as path. This
- * will be removed later. */
- backingStore->type = VIR_STORAGE_TYPE_NETWORK;
-
- if (VIR_STRDUP(backingStore->path, src->backingStoreRaw) < 0)
- 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 (virStorageFileGetMetadataRecurse(backingStore,
- backingStore->path,
- uid, gid, allow_probe,
- cycle) < 0) {
- /* if we fail somewhere midway, just accept and return a
- * broken chain */
- ret = 0;
- goto cleanup;
- }
-
- src->backingStore = backingStore;
- backingStore = NULL;
- ret = 0;
-
- cleanup:
- 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.
- *
- * Caller MUST free result after use via virStorageSourceFree.
- */
-int
-virStorageFileGetMetadata(virStorageSourcePtr src,
- uid_t uid, gid_t gid,
- bool allow_probe)
-{
- VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d",
- src->path, src->format, (int)uid, (int)gid, allow_probe);
-
- virHashTablePtr cycle = NULL;
- char *canonPath = NULL;
- int ret = -1;
-
- if (!(cycle = virHashCreate(5, NULL)))
- return -1;
-
- if (virStorageSourceGetActualType(src) != VIR_STORAGE_TYPE_NETWORK) {
- if (!(canonPath = canonicalize_file_name(src->path))) {
- virReportSystemError(errno, _("unable to resolve '%s'"),
- src->path);
- goto cleanup;
- }
-
- if (!src->relPath &&
- VIR_STRDUP(src->relPath, src->path) < 0)
- goto cleanup;
-
- if (!src->relDir &&
- !(src->relDir = mdir_name(src->path))) {
- virReportOOMError();
- goto cleanup;
- }
- } else {
- /* TODO: currently unimplemented for non-local storage */
- ret = 0;
- goto cleanup;
- }
-
- if (src->format <= VIR_STORAGE_FILE_NONE)
- src->format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
-
- ret = virStorageFileGetMetadataRecurse(src, canonPath, uid, gid,
- allow_probe, cycle);
-
- cleanup:
- VIR_FREE(canonPath);
- virHashFree(cycle);
- return ret;
-}
-
/**
* virStorageFileChainCheckBroken
*
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 082ff5b..c1babdc 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -265,10 +265,9 @@ struct _virStorageSource {
int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
-int virStorageFileGetMetadata(virStorageSourcePtr src,
- uid_t uid, gid_t gid,
- bool allow_probe)
- ATTRIBUTE_NONNULL(1);
+int virStorageFileGetMetadataFromFDInternal(virStorageSourcePtr meta,
+ int fd,
+ int *backingFormat);
virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
int fd,
int format);
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 5ef8940..4d24822 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -869,7 +869,12 @@ virstringtest_LDADD = $(LDADDS)
virstoragetest_SOURCES = \
virstoragetest.c testutils.h testutils.c
-virstoragetest_LDADD = $(LDADDS)
+virstoragetest_LDADD = $(LDADDS) \
+ ../src/libvirt.la \
+ ../src/libvirt_conf.la \
+ ../src/libvirt_util.la \
+ ../src/libvirt_driver_storage_impl.la \
+ ../gnulib/lib/libgnu.la
viridentitytest_SOURCES = \
viridentitytest.c testutils.h testutils.c
diff --git a/tests/virstoragetest.c b/tests/virstoragetest.c
index d49098e..fb96c71 100644
--- a/tests/virstoragetest.c
+++ b/tests/virstoragetest.c
@@ -31,6 +31,8 @@
#include "virstring.h"
#include "dirname.h"
+#include "storage/storage_driver.h"
+
#define VIR_FROM_THIS VIR_FROM_NONE
VIR_LOG_INIT("tests.storagetest");
--
1.9.3
More information about the libvir-list
mailing list