[libvirt] [PATCH 2/5] conf: fix detection of infinite backing loop
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.
* src/util/virstoragefile.c (virStorageFileGetMetadataRecurse):
Mark chain broken if OOM or infinite loop occurs.
Signed-off-by: Eric Blake <eblake(a)redhat.com>
---
src/util/virstoragefile.c | 24 +++++++++++++++++++-----
1 file changed, 19 insertions(+), 5 deletions(-)
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 017717c..39e4c19 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1074,15 +1074,21 @@ virStorageFileGetMetadataRecurse(const char *path, const char
*directory,
path, format, (int)uid, (int)gid, allow_probe);
virStorageFileMetadataPtr ret = NULL;
+ char *canonical = canonicalize_file_name(path);
- if (virHashLookup(cycle, path)) {
+ /* Hash by canonical name */
+ if (!canonical) {
+ virReportOOMError();
+ goto cleanup;
+ }
+ if (virHashLookup(cycle, canonical)) {
virReportError(VIR_ERR_INTERNAL_ERROR,
_("backing store for %s is self-referential"),
path);
- return NULL;
+ goto cleanup;
}
- if (virHashAddEntry(cycle, path, (void *)1) < 0)
- return NULL;
+ if (virHashAddEntry(cycle, canonical, (void *)1) < 0)
+ goto cleanup;
if ((fd = virFileOpenAs(path, O_RDONLY, 0, uid, gid, 0)) < 0) {
virReportSystemError(-fd, _("Failed to open file '%s'"),
path);
@@ -1106,8 +1112,15 @@ virStorageFileGetMetadataRecurse(const char *path, const char
*directory,
uid, gid,
allow_probe,
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);
+ }
}
+ cleanup:
+ VIR_FREE(canonical);
return ret;
}
@@ -1176,7 +1189,8 @@ virStorageFileChainGetBroken(virStorageFileMetadataPtr chain,
tmp = chain;
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)
break;
if (!tmp->backingStore) {
--
1.9.0