From 6752bc2add9c2d1d87492c227df98bf533c3333f Mon Sep 17 00:00:00 2001
From: Eric Blake <eblake@redhat.com>
Date: Thu, 10 Apr 2014 19:03:01 -0600
Subject: [PATCH] conf: report error on chain lookup failure

The chain lookup function was inconsistent on whether it left
a message in the log when looking up a name that is not found
on the chain (leaving a message for OOM or if name was
relative but not part of the chain), and could litter the log
even when successful (when name was relative but deep in the
chain, use of virFindBackingFile early in the chain would complain
about a file not found).  It's easier to make the function
consistently emit a message exactly once on failure, and to let
all callers rely on the clean semantics.

* src/util/virstoragefile.c (virStorageFileChainLookup): Always
report error on failure.  Simplify relative lookups.
* src/qemu/qemu_driver.c (qemuDomainBlockCommit): Avoid
overwriting error.

Signed-off-by: Eric Blake <eblake@redhat.com>
---
 src/qemu/qemu_driver.c    | 15 ++++-----------
 src/util/virstoragefile.c | 22 ++++++++++++++--------
 2 files changed, 18 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index b0c6a741cb..fed7d1cf9a 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15346,9 +15346,6 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
                                                        disk->src.path,
                                                        top, &top_meta,
                                                        &top_parent))) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("could not find top '%s' in chain for '%s'"),
-                       top, path);
         goto endjob;
     }
     if (!top_meta || !top_meta->backingStore) {
@@ -15357,16 +15354,12 @@ qemuDomainBlockCommit(virDomainPtr dom, const char *path, const char *base,
                        top_canon, path);
         goto endjob;
     }
-    if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW)) {
+    if (!base && (flags & VIR_DOMAIN_BLOCK_COMMIT_SHALLOW))
         base_canon = top_meta->backingStore;
-    } else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon,
-                                                        base, NULL, NULL))) {
-        virReportError(VIR_ERR_INVALID_ARG,
-                       _("could not find base '%s' below '%s' in chain "
-                         "for '%s'"),
-                       base ? base : "(default)", top_canon, path);
+    else if (!(base_canon = virStorageFileChainLookup(top_meta, top_canon,
+                                                      base, NULL, NULL)))
         goto endjob;
-    }
+
     /* Note that this code exploits the fact that
      * virStorageFileChainLookup guarantees a simple pointer
      * comparison will work, rather than needing full-blown STREQ.  */
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 201391482d..77cc8781f9 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -1537,7 +1537,8 @@ int virStorageFileGetSCSIKey(const char *path,
  * backing element is not a file).  If PARENT is not NULL, set *PARENT
  * to the preferred name of the parent (or to NULL if NAME matches
  * START).  Since the results point within CHAIN, they must not be
- * independently freed.  */
+ * independently freed.  Reports an error and returns NULL if NAME is
+ * not found.  */
 const char *
 virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
                           const char *name, virStorageFileMetadataPtr *meta,
@@ -1570,15 +1571,12 @@ virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
                    STREQ(name, owner->backingStore)) {
             break;
         } else if (virStorageIsFile(owner->backingStore)) {
-            char *absName = NULL;
-            if (virFindBackingFile(owner->directory, name,
-                                   NULL, &absName) < 0)
+            int result = virFileRelLinkPointsTo(owner->directory, name,
+                                                owner->backingStore);
+            if (result < 0)
                 goto error;
-            if (absName && STREQ(absName, owner->backingStore)) {
-                VIR_FREE(absName);
+            if (result > 0)
                 break;
-            }
-            VIR_FREE(absName);
         }
         *parent = owner->backingStore;
         owner = owner->backingMeta;
@@ -1590,6 +1588,14 @@ virStorageFileChainLookup(virStorageFileMetadataPtr chain, const char *start,
     return owner->backingStore;
 
  error:
+    if (name)
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("could not find image '%s' in chain for '%s'"),
+                       name, start);
+    else
+        virReportError(VIR_ERR_INVALID_ARG,
+                       _("could not find base image in chain for '%s'"),
+                       start);
     *parent = NULL;
     if (meta)
         *meta = NULL;