From 0d44d2876afc9bc10ef6cbc8597c8704041be559 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 22 Mar 2019 15:08:36 +0100
Subject: [PATCH] virfile: Make virFileGetXAttr report errors
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The way that security drivers use XATTR is kind of verbose. If
error reporting was left for caller then the caller would end up
even more verbose.

There are two places where we do not want to report error if
virFileGetXAttr fails. Therefore virFileGetXAttrQuiet is
introduced as an alternative that doesn't report errors.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Cole Robinson <crobinso@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/libvirt_private.syms     |  1 +
 src/security/security_util.c |  4 ++--
 src/util/virfile.c           | 42 ++++++++++++++++++++++++++++++------
 src/util/virfile.h           |  5 +++++
 tests/qemusecuritymock.c     |  6 +++---
 5 files changed, 46 insertions(+), 12 deletions(-)

diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 34937adc5d..cf993f0c9c 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -1908,6 +1908,7 @@ virFileGetHugepageSize;
 virFileGetMountReverseSubtree;
 virFileGetMountSubtree;
 virFileGetXAttr;
+virFileGetXAttrQuiet;
 virFileInData;
 virFileIsAbsPath;
 virFileIsCDROM;
diff --git a/src/security/security_util.c b/src/security/security_util.c
index bfa78c6cca..f09a18a623 100644
--- a/src/security/security_util.c
+++ b/src/security/security_util.c
@@ -123,7 +123,7 @@ virSecurityGetRememberedLabel(const char *name,
     if (!(ref_name = virSecurityGetRefCountAttrName(name)))
         goto cleanup;
 
-    if (virFileGetXAttr(path, ref_name, &value) < 0) {
+    if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
         if (errno == ENOSYS || errno == ENODATA || errno == ENOTSUP) {
             ret = 0;
         } else {
@@ -208,7 +208,7 @@ virSecuritySetRememberedLabel(const char *name,
     if (!(ref_name = virSecurityGetRefCountAttrName(name)))
         goto cleanup;
 
-    if (virFileGetXAttr(path, ref_name, &value) < 0) {
+    if (virFileGetXAttrQuiet(path, ref_name, &value) < 0) {
         if (errno == ENOSYS || errno == ENOTSUP) {
             ret = 0;
             goto cleanup;
diff --git a/src/util/virfile.c b/src/util/virfile.c
index f7415cf633..00f69dce69 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -4364,7 +4364,7 @@ virFileWaitForExists(const char *path,
 
 #if HAVE_LIBATTR
 /**
- * virFileGetXAttr;
+ * virFileGetXAttrQuiet;
  * @path: a filename
  * @name: name of xattr
  * @value: read value
@@ -4376,9 +4376,9 @@ virFileWaitForExists(const char *path,
  *         -1 otherwise (with errno set).
  */
 int
-virFileGetXAttr(const char *path,
-                const char *name,
-                char **value)
+virFileGetXAttrQuiet(const char *path,
+                     const char *name,
+                     char **value)
 {
     char *buf = NULL;
     int ret = -1;
@@ -4451,9 +4451,9 @@ virFileRemoveXAttr(const char *path,
 #else /* !HAVE_LIBATTR */
 
 int
-virFileGetXAttr(const char *path ATTRIBUTE_UNUSED,
-                const char *name ATTRIBUTE_UNUSED,
-                char **value ATTRIBUTE_UNUSED)
+virFileGetXAttrQuiet(const char *path ATTRIBUTE_UNUSED,
+                     const char *name ATTRIBUTE_UNUSED,
+                     char **value ATTRIBUTE_UNUSED)
 {
     errno = ENOSYS;
     return -1;
@@ -4477,3 +4477,31 @@ virFileRemoveXAttr(const char *path ATTRIBUTE_UNUSED,
 }
 
 #endif /* HAVE_LIBATTR */
+
+/**
+ * virFileGetXAttr;
+ * @path: a filename
+ * @name: name of xattr
+ * @value: read value
+ *
+ * Reads xattr with @name for given @path and stores it into
+ * @value. Caller is responsible for freeing @value.
+ *
+ * Returns: 0 on success,
+ *         -1 otherwise (with errno set AND error reported).
+ */
+int
+virFileGetXAttr(const char *path,
+                const char *name,
+                char **value)
+{
+    int ret;
+
+    if ((ret = virFileGetXAttrQuiet(path, name, value)) < 0) {
+        virReportSystemError(errno,
+                             _("Unable to get XATTR %s on %s"),
+                             name, path);
+    }
+
+    return ret;
+}
diff --git a/src/util/virfile.h b/src/util/virfile.h
index 094c92e2b9..c2d1b36c85 100644
--- a/src/util/virfile.h
+++ b/src/util/virfile.h
@@ -383,6 +383,11 @@ int virFileGetXAttr(const char *path,
                     char **value)
     ATTRIBUTE_NOINLINE;
 
+int virFileGetXAttrQuiet(const char *path,
+                         const char *name,
+                         char **value)
+    ATTRIBUTE_NOINLINE;
+
 int virFileSetXAttr(const char *path,
                     const char *name,
                     const char *value)
diff --git a/tests/qemusecuritymock.c b/tests/qemusecuritymock.c
index 815bd44d76..e219a9f023 100644
--- a/tests/qemusecuritymock.c
+++ b/tests/qemusecuritymock.c
@@ -126,9 +126,9 @@ get_key(const char *path,
 
 
 int
-virFileGetXAttr(const char *path,
-                const char *name,
-                char **value)
+virFileGetXAttrQuiet(const char *path,
+                     const char *name,
+                     char **value)
 {
     int ret = -1;
     char *key;