[PATCH 1/3] virSecurityManagerMetadataLock: Store locked paths

[Michal Privoznik]

So far, in the lock state we are storing only the file
descriptors of the files we've locked. Therefore, when unlocking
them and something does wrong the only thing we can report is FD
number, which is not user friendly at all. But if we store paths
among with FDs we can do better error reporting.

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/security/security_manager.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/src/security/security_manager.c b/src/security/security_manager.c
index fe9def7fb9..aea8cc2fb9 100644
--- a/src/security/security_manager.c
+++ b/src/security/security_manager.c
@@ -1246,8 +1246,9 @@ virSecurityManagerRestoreTPMLabels(virSecurityManagerPtr mgr,
 
 
 struct _virSecurityManagerMetadataLockState {
-    size_t nfds;
+    size_t nfds; /* Captures size of both @fds and @paths */
     int *fds;
+    const char **paths;
 };
 
 
@@ -1278,6 +1279,7 @@ cmpstringp(const void *p1, const void *p2)
  *
  * Lock passed @paths for metadata change. The returned state
  * should be passed to virSecurityManagerMetadataUnlock.
+ * Passed @paths must not be freed until the corresponding unlock call.
  *
  * NOTE: this function is not thread safe (because of usage of
  * POSIX locks).
@@ -1293,9 +1295,11 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
     size_t i = 0;
     size_t nfds = 0;
     int *fds = NULL;
+    const char **locked_paths = NULL;
     virSecurityManagerMetadataLockStatePtr ret = NULL;
 
-    if (VIR_ALLOC_N(fds, npaths) < 0)
+    if (VIR_ALLOC_N(fds, npaths) < 0 ||
+        VIR_ALLOC_N(locked_paths, npaths) < 0)
         return NULL;
 
     /* Sort paths to lock in order to avoid deadlocks with other
@@ -1374,12 +1378,14 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
             break;
         } while (1);
 
+        locked_paths[nfds] = p;
         VIR_APPEND_ELEMENT_COPY_INPLACE(fds, nfds, fd);
     }
 
     if (VIR_ALLOC(ret) < 0)
         goto cleanup;
 
+    ret->paths = g_steal_pointer(&locked_paths);
     ret->fds = g_steal_pointer(&fds);
     ret->nfds = nfds;
     nfds = 0;
@@ -1388,6 +1394,7 @@ virSecurityManagerMetadataLock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
     for (i = nfds; i > 0; i--)
         VIR_FORCE_CLOSE(fds[i - 1]);
     VIR_FREE(fds);
+    VIR_FREE(locked_paths);
     return ret;
 }
 
@@ -1403,21 +1410,23 @@ virSecurityManagerMetadataUnlock(virSecurityManagerPtr mgr G_GNUC_UNUSED,
 
     for (i = 0; i < (*state)->nfds; i++) {
         char ebuf[1024];
+        const char *path = (*state)->paths[i];
         int fd = (*state)->fds[i];
 
         /* Technically, unlock is not needed because it will
          * happen on VIR_CLOSE() anyway. But let's play it nice. */
         if (virFileUnlock(fd, METADATA_OFFSET, METADATA_LEN) < 0) {
-            VIR_WARN("Unable to unlock fd %d: %s",
-                     fd, virStrerror(errno, ebuf, sizeof(ebuf)));
+            VIR_WARN("Unable to unlock fd %d path %s: %s",
+                     fd, path, virStrerror(errno, ebuf, sizeof(ebuf)));
         }
 
         if (VIR_CLOSE(fd) < 0) {
-            VIR_WARN("Unable to close fd %d: %s",
-                     fd, virStrerror(errno, ebuf, sizeof(ebuf)));
+            VIR_WARN("Unable to close fd %d path %s: %s",
+                     fd, path, virStrerror(errno, ebuf, sizeof(ebuf)));
         }
     }
 
     VIR_FREE((*state)->fds);
+    VIR_FREE((*state)->paths);
     VIR_FREE(*state);
 }
-- 
2.24.1