External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-12-22 05:35:25 +00:00
Code Issues Packages Projects Releases Wiki Activity

util: storage: Invert the way recursive metadata retrieval works

Browse Source 
To avoid having the root of a backing chain present twice in the list we
need to invert the working of virStorageFileGetMetadataRecurse.

Until now the recursive worker created a new backing chain element from
the name and other information passed as arguments. This required us to
pass the data of the parent in a deconstructed way and the worker
created a new entry for the parent.

This patch converts this function so that it just fills in metadata
about the parent and creates a backing chain element from those. This
removes the duplication of the first element.

To avoid breaking the test suite, virstoragetest now calls a wrapper
that creates the parent structure explicitly and pre-fills it with the
test data with same function signature as previously used.
This commit is contained in:
Peter Krempa 2014-04-18 14:49:54 +02:00
parent cc92ee32cd
commit 8823272d41
7 changed files with 165 additions and 126 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/conf/domain_conf.c
View File

@ -18537,9 +18537,8 @@ virDomainDiskDefForeachPath(virDomainDiskDefPtr disk,
if (iter(disk, path, 0, opaque) < 0)
goto cleanup;
/* XXX: temporarily we need to select the second element of the backing
* chain to start as the first is the copy of the disk itself. */
tmp = disk->src.backingStore ? disk->src.backingStore->backingStore : NULL;
tmp = disk->src.backingStore;
while (tmp && virStorageIsFile(tmp->path)) {
if (!ignoreOpenFailure && tmp->backingStoreRaw && !tmp->backingStore) {
virReportError(VIR_ERR_INTERNAL_ERROR,

12
src/qemu/qemu_domain.c
View File

@ -2239,10 +2239,10 @@ qemuDiskChainCheckBroken(virDomainDiskDefPtr disk)
{
char *brokenFile = NULL;
if (!virDomainDiskGetSource(disk) || !disk->src.backingStore)
if (!virDomainDiskGetSource(disk))
return 0;
if (virStorageFileChainGetBroken(disk->src.backingStore, &brokenFile) < 0)
if (virStorageFileChainGetBroken(&disk->src, &brokenFile) < 0)
return -1;
if (brokenFile) {
@ -2419,11 +2419,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
qemuDomainGetImageIds(cfg, vm, disk, &uid, &gid);
disk->src.backingStore = virStorageFileGetMetadata(src,
virDomainDiskGetFormat(disk),
uid, gid,
cfg->allowDiskFormatProbing);
if (!disk->src.backingStore)
if (virStorageFileGetMetadata(&disk->src,
uid, gid,
cfg->allowDiskFormatProbing) < 0)
ret = -1;
cleanup:

6
src/qemu/qemu_driver.c
View File

@ -15123,7 +15123,7 @@ qemuDomainBlockCopy(virDomainObjPtr vm,
if ((flags & VIR_DOMAIN_BLOCK_REBASE_SHALLOW) &&
STREQ_NULLABLE(format, "raw") &&
disk->src.backingStore->backingStore->path) {
disk->src.backingStore->path) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
_("disk '%s' has backing file, so raw shallow copy "
"is not possible"),
@ -15330,8 +15330,8 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
if (!top) {
top_canon = disk->src.path;
top_meta = disk->src.backingStore;
} else if (!(top_canon = virStorageFileChainLookup(disk->src.backingStore,
top_meta = &disk->src;
} else if (!(top_canon = virStorageFileChainLookup(&disk->src,
top, &top_meta,
&top_parent))) {
goto endjob;

7
src/security/virt-aa-helper.c
View File

@ -943,18 +943,15 @@ get_files(vahControl * ctl)
for (i = 0; i < ctl->def->ndisks; i++) {
virDomainDiskDefPtr disk = ctl->def->disks[i];
const char *src = virDomainDiskGetSource(disk);
if (!src)
if (!virDomainDiskGetSource(disk))
continue;
/* XXX - if we knew the qemu user:group here we could send it in
* so that the open could be re-tried as that user:group.
*/
if (!disk->src.backingStore) {
bool probe = ctl->allowDiskFormatProbing;
disk->src.backingStore = virStorageFileGetMetadata(src,
virDomainDiskGetFormat(disk),
-1, -1, probe);
virStorageFileGetMetadata(&disk->src, -1, -1, probe);
}
/* XXX passing ignoreOpenFailure = true to get back to the behavior

199
src/util/virstoragefile.c
View File

@ -1110,105 +1110,112 @@ virStorageFileGetMetadataFromFD(const char *path,
/* Recursive workhorse for virStorageFileGetMetadata. */
static int
virStorageFileGetMetadataRecurse(const char *path, const char *canonPath,
const char *directory,
int format, uid_t uid, gid_t gid,
bool allow_probe, virHashTablePtr cycle,
virStorageSourcePtr meta)
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;
char *backingPath = NULL;
char *backingDirectory = NULL;
VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d",
path, canonPath, NULLSTR(directory), format,
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"),
path);
src->path);
return -1;
}
if (virHashAddEntry(cycle, canonPath, (void *)1) < 0)
return -1;
if (virStorageIsFile(path)) {
if (VIR_STRDUP(meta->relPath, path) < 0)
return -1;
if (VIR_STRDUP(meta->path, canonPath) < 0)
return -1;
if (VIR_STRDUP(meta->relDir, directory) < 0)
return -1;
meta->format = format;
if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) {
virReportSystemError(-fd, _("Failed to open file '%s'"), path);
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;
}
ret = virStorageFileGetMetadataFromFDInternal(meta, fd, &backingFormat);
if (virStorageFileGetMetadataFromFDInternal(src, fd,
&backingFormat) < 0) {
VIR_FORCE_CLOSE(fd);
return -1;
}
if (VIR_CLOSE(fd) < 0)
VIR_WARN("could not close file %s", path);
VIR_WARN("could not close file %s", src->path);
} else {
/* FIXME: when the proper storage drivers are compiled in, it
* would be nice to read metadata from the network storage to
* allow for non-raw images. */
if (VIR_STRDUP(meta->relPath, path) < 0)
return -1;
if (VIR_STRDUP(meta->path, path) < 0)
return -1;
meta->type = VIR_STORAGE_TYPE_NETWORK;
meta->format = VIR_STORAGE_FILE_RAW;
/* 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;
}
if (ret == 0 && meta->backingStoreRaw) {
virStorageSourcePtr backing;
src->backingStore = backingStore;
backingStore = NULL;
ret = 0;
if (virStorageIsFile(meta->backingStoreRaw)) {
if (virFindBackingFile(directory,
meta->backingStoreRaw,
&backingDirectory,
&backingPath) < 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.",
meta->backingStoreRaw, path);
return 0;
}
} else {
if (VIR_STRDUP(backingPath, meta->backingStoreRaw) < 0)
return -1;
}
if (backingFormat == VIR_STORAGE_FILE_AUTO && !allow_probe)
backingFormat = VIR_STORAGE_FILE_RAW;
else if (backingFormat == VIR_STORAGE_FILE_AUTO_SAFE)
backingFormat = VIR_STORAGE_FILE_AUTO;
if (VIR_ALLOC(backing) < 0 ||
virStorageFileGetMetadataRecurse(meta->backingStoreRaw,
backingPath,
backingDirectory, backingFormat,
uid, gid, allow_probe,
cycle, backing) < 0) {
/* If we failed to get backing data, mark the chain broken */
virStorageSourceFree(backing);
} else {
meta->backingStore = backing;
}
}
VIR_FREE(backingDirectory);
VIR_FREE(backingPath);
cleanup:
virStorageSourceFree(backingStore);
return ret;
}
/**
* virStorageFileGetMetadata:
*
@ -1226,51 +1233,51 @@ virStorageFileGetMetadataRecurse(const char *path, const char *canonPath,
*
* Caller MUST free result after use via virStorageSourceFree.
*/
virStorageSourcePtr
virStorageFileGetMetadata(const char *path, int format,
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",
path, format, (int)uid, (int)gid, allow_probe);
src->path, src->format, (int)uid, (int)gid, allow_probe);
virHashTablePtr cycle = virHashCreate(5, NULL);
virStorageSourcePtr meta = NULL;
virStorageSourcePtr ret = NULL;
virHashTablePtr cycle = NULL;
char *canonPath = NULL;
char *directory = NULL;
int ret = -1;
if (!cycle)
return NULL;
if (!(cycle = virHashCreate(5, NULL)))
return -1;
if (virStorageIsFile(path)) {
if (!(canonPath = canonicalize_file_name(path))) {
virReportSystemError(errno, _("unable to resolve '%s'"), path);
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 (!(directory = mdir_name(path))) {
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 if (VIR_STRDUP(canonPath, path) < 0) {
} else {
/* TODO: currently unimplemented for non-local storage */
ret = 0;
goto cleanup;
}
if (VIR_ALLOC(meta) < 0)
goto cleanup;
if (format <= VIR_STORAGE_FILE_NONE)
format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
if (virStorageFileGetMetadataRecurse(path, canonPath, directory, format,
uid, gid, allow_probe, cycle,
meta) < 0)
goto cleanup;
ret = meta;
meta = NULL;
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:
virStorageSourceFree(meta);
VIR_FREE(canonPath);
VIR_FREE(directory);
virHashFree(cycle);
return ret;
}

7
src/util/virstoragefile.h
View File

@ -263,10 +263,9 @@ int virStorageFileProbeFormat(const char *path, uid_t uid, gid_t gid);
int virStorageFileProbeFormatFromBuf(const char *path, char *buf,
size_t buflen);
virStorageSourcePtr virStorageFileGetMetadata(const char *path,
int format,
uid_t uid, gid_t gid,
bool allow_probe)
int virStorageFileGetMetadata(virStorageSourcePtr src,
uid_t uid, gid_t gid,
bool allow_probe)
ATTRIBUTE_NONNULL(1);
virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
int fd,

55
tests/virstoragetest.c
View File

@ -29,6 +29,7 @@
#include "virlog.h"
#include "virstoragefile.h"
#include "virstring.h"
#include "dirname.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@ -88,6 +89,44 @@ testCleanupImages(void)
virFileDeleteTree(datadir);
}
static virStorageSourcePtr
testStorageFileGetMetadata(const char *path,
int format,
uid_t uid, gid_t gid,
bool allow_probe)
{
virStorageSourcePtr ret = NULL;
if (VIR_ALLOC(ret) < 0)
return NULL;
ret->type = VIR_STORAGE_TYPE_FILE;
ret->format = format;
if (VIR_STRDUP(ret->relPath, path) < 0)
goto error;
if (!(ret->relDir = mdir_name(path))) {
virReportOOMError();
goto error;
}
if (!(ret->path = canonicalize_file_name(path))) {
virReportError(VIR_ERR_INTERNAL_ERROR, "failed to resolve '%s'", path);
goto error;
}
if (virStorageFileGetMetadata(ret, uid, gid, allow_probe) < 0)
goto error;
return ret;
error:
virStorageSourceFree(ret);
return NULL;
}
static int
testPrepImages(void)
{
@ -272,8 +311,8 @@ testStorageChain(const void *args)
char *broken = NULL;
bool isAbs = !!(data->flags & ABS_START);
meta = virStorageFileGetMetadata(data->start, data->format, -1, -1,
(data->flags & ALLOW_PROBE) != 0);
meta = testStorageFileGetMetadata(data->start, data->format, -1, -1,
(data->flags & ALLOW_PROBE) != 0);
if (!meta) {
if (data->flags & EXP_FAIL) {
virResetLastError();
@ -825,8 +864,8 @@ mymain(void)
ret = -1;
/* Test behavior of chain lookups, absolute backing from relative start */
chain = virStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2,
-1, -1, false);
chain = testStorageFileGetMetadata("wrap", VIR_STORAGE_FILE_QCOW2,
-1, -1, false);
if (!chain) {
ret = -1;
goto cleanup;
@ -870,8 +909,8 @@ mymain(void)
/* Test behavior of chain lookups, relative backing from absolute start */
virStorageSourceFree(chain);
chain = virStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2,
-1, -1, false);
chain = testStorageFileGetMetadata(abswrap, VIR_STORAGE_FILE_QCOW2,
-1, -1, false);
if (!chain) {
ret = -1;
goto cleanup;
@ -900,8 +939,8 @@ mymain(void)
/* Test behavior of chain lookups, relative backing */
virStorageSourceFree(chain);
chain = virStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2,
-1, -1, false);
chain = testStorageFileGetMetadata("sub/link2", VIR_STORAGE_FILE_QCOW2,
-1, -1, false);
if (!chain) {
ret = -1;
goto cleanup;