External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2025-01-22 04:25:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

conf: fix detection of infinite backing loop

Browse Source 
While trying to refactor the backing file chain, I noticed that
if you have a self-referential qcow2 file via a relative name:

qemu-img create -f qcow2 loop 10M
qemu-img rebase -u -f qcow2 -F qcow2 -b loop loop

then libvirt was creating a chain 2 deep before realizing it
had hit a loop; furthermore, virStorageFileChainCheckBroken
was not identifying the chain as broken.  With this patch,
the loop is detected when the chain is only 1 deep; still
enough for storage volume XML to display the file, but now
with a proper error report about where the loop was found.

This patch adds a parameter to virStorageFileGetMetadataRecurse,
so that errors at the top of the chain remain unchanged; messages
issued for backing files now use the name provided by the user
instead of the canonical name (for VDSM, which uses relative
symlinks to device mapper block devices, this is actually more
useful).

* src/util/virstoragefile.c (virStorageFileGetMetadataRecurse):
Add parameter, require canonical path up front.  Mark chain
broken on OOM or loop detection.
(virStorageFileGetMetadata): Pass in canonical name.

Signed-off-by: Eric Blake <eblake@redhat.com>
This commit is contained in:
Eric Blake 2014-04-04 18:05:22 -06:00
parent 2fe658ad90
commit af095bfa0d
1 changed files with 28 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

40
src/util/virstoragefile.c
View File

@ -1065,26 +1065,28 @@ virStorageFileGetMetadataFromFD(const char *path,
/* Recursive workhorse for virStorageFileGetMetadata. */ /* Recursive workhorse for virStorageFileGetMetadata. */
static virStorageFileMetadataPtr static virStorageFileMetadataPtr
virStorageFileGetMetadataRecurse(const char *path, const char *directory, virStorageFileGetMetadataRecurse(const char *path, const char *canonPath,
const char *directory,
int format, uid_t uid, gid_t gid, int format, uid_t uid, gid_t gid,
bool allow_probe, virHashTablePtr cycle) bool allow_probe, virHashTablePtr cycle)
{ {
int fd; int fd;
VIR_DEBUG("path=%s format=%d uid=%d gid=%d probe=%d", VIR_DEBUG("path=%s canonPath=%s dir=%s format=%d uid=%d gid=%d probe=%d",
path, format, (int)uid, (int)gid, allow_probe); path, canonPath, NULLSTR(directory), format,
(int)uid, (int)gid, allow_probe);
virStorageFileMetadataPtr ret = NULL; virStorageFileMetadataPtr ret = NULL;
if (virHashLookup(cycle, path)) { if (virHashLookup(cycle, canonPath)) {
virReportError(VIR_ERR_INTERNAL_ERROR, virReportError(VIR_ERR_INTERNAL_ERROR,
_("backing store for %s is self-referential"), _("backing store for %s is self-referential"),
path); path);
return NULL; return NULL;
} }
if (virHashAddEntry(cycle, path, (void *)1) < 0) if (virHashAddEntry(cycle, canonPath, (void *)1) < 0)
return NULL; return NULL;
if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) { if ((fd = virFileOpenAs(canonPath, O_RDONLY, 0, uid, gid, 0)) < 0) {
virReportSystemError(-fd, _("Failed to open file '%s'"), path); virReportSystemError(-fd, _("Failed to open file '%s'"), path);
return NULL; return NULL;
} }
@ -1100,14 +1102,19 @@ virStorageFileGetMetadataRecurse(const char *path, const char *directory,
else if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE) else if (ret->backingStoreFormat == VIR_STORAGE_FILE_AUTO_SAFE)
ret->backingStoreFormat = VIR_STORAGE_FILE_AUTO; ret->backingStoreFormat = VIR_STORAGE_FILE_AUTO;
format = ret->backingStoreFormat; format = ret->backingStoreFormat;
ret->backingMeta = virStorageFileGetMetadataRecurse(ret->backingStore, ret->backingMeta = virStorageFileGetMetadataRecurse(ret->backingStoreRaw,
ret->backingStore,
ret->directory, ret->directory,
format, format,
uid, gid, uid, gid,
allow_probe, allow_probe,
cycle); cycle);
if (!ret->backingMeta) {
/* If we failed to get backing data, mark the chain broken */
ret->backingStoreFormat = VIR_STORAGE_FILE_NONE;
VIR_FREE(ret->backingStore);
}
} }
return ret; return ret;
} }
@ -1142,15 +1149,23 @@ virStorageFileGetMetadata(const char *path, int format,
path, format, (int)uid, (int)gid, allow_probe); path, format, (int)uid, (int)gid, allow_probe);
virHashTablePtr cycle = virHashCreate(5, NULL); virHashTablePtr cycle = virHashCreate(5, NULL);
virStorageFileMetadataPtr ret; virStorageFileMetadataPtr ret = NULL;
char *canonPath;
if (!cycle) if (!cycle)
return NULL; return NULL;
if (!(canonPath = canonicalize_file_name(path))) {
virReportSystemError(errno, _("unable to resolve '%s'"), path);
goto cleanup;
}
if (format <= VIR_STORAGE_FILE_NONE) if (format <= VIR_STORAGE_FILE_NONE)
format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW; format = allow_probe ? VIR_STORAGE_FILE_AUTO : VIR_STORAGE_FILE_RAW;
ret = virStorageFileGetMetadataRecurse(path, NULL, format, uid, gid, ret = virStorageFileGetMetadataRecurse(path, canonPath, NULL, format,
allow_probe, cycle); uid, gid, allow_probe, cycle);
cleanup:
VIR_FREE(canonPath);
virHashFree(cycle); virHashFree(cycle);
return ret; return ret;
} }
@ -1176,7 +1191,8 @@ virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
tmp = chain; tmp = chain;
while (tmp) { while (tmp) {
/* Break if no backing store or backing store is not file */ /* Break if no backing store, backing store is not file, or
* other problem such as infinite loop */
if (!tmp->backingStoreRaw) if (!tmp->backingStoreRaw)
break; break;
if (!tmp->backingStore) { if (!tmp->backingStore) {