From d415286e93faf2f9c4fa5c5c589aa8d3f433d01d Mon Sep 17 00:00:00 2001
Message-Id: <d415286e93faf2f9c4fa5c5c589aa8d3f433d01d@dist-git>
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 19 Jun 2020 11:43:13 +0200
Subject: [PATCH] qemu: Create multipath targets for PRs
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If a disk has persistent reservations enabled, qemu-pr-helper
might open not only /dev/mapper/control but also individual
targets of the multipath device. We are already querying for them
in CGroups, but now we have to create them in the namespace too.
This was brought up in [1].

1: https://bugzilla.redhat.com/show_bug.cgi?id=1711045#c61

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Tested-by: Lin Ma <LMa@suse.com>
Reviewed-by: Jim Fehlig <jfehlig@suse.com>
(cherry picked from commit a30078cb832646177defd256e77c632905f1e6d0)

https://bugzilla.redhat.com/show_bug.cgi?id=1839992

Conflicts:
- src/qemu/qemu_domain.c: Well, the upstream has NVMe and
  downstream doesn't. So the functions I'm modifying look
  different in the upstream. Tough luck.
- src/util/virdevmapper.h: The upstream moved to gnulib macros
  (G_GNUC_NO_INLINE in this case). But since I'm removing changes
  to qemuhotplugtest below, this particular change wasn't needed.
  The include of "internal.h" is though (because of next commit).
- tests/qemuhotplugmock.c:
- tests/qemuhotplugtest.c: I'm completely dropping these two
  changes because the upstream went a long way since and these
  are only tests.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <f33e5c3f08bb8ea4e5fb65d6ee3e7542cdddf3b0.1592559697.git.mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---
 src/qemu/qemu_domain.c  | 59 ++++++++++++++++++++++++++++-------------
 src/util/virdevmapper.h |  2 ++
 2 files changed, 42 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index b4792c9476..489e20a28a 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -57,6 +57,7 @@
 #include "secret_util.h"
 #include "logging/log_manager.h"
 #include "locking/domain_lock.h"
+#include "virdevmapper.h"
 
 #ifdef MAJOR_IN_MKDEV
 # include <sys/mkdev.h>
@@ -11325,13 +11326,29 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg ATTRIBUTE_UNUSED,
     int ret = -1;
 
     for (next = disk->src; virStorageSourceIsBacking(next); next = next->backingStore) {
+        VIR_AUTOSTRINGLIST targetPaths = NULL;
+        size_t i;
+
         if (!next->path || !virStorageSourceIsLocalStorage(next)) {
             /* Not creating device. Just continue. */
             continue;
         }
 
         if (qemuDomainCreateDevice(next->path, data, false) < 0)
-            goto cleanup;
+            return -1;
+
+        if (virDevMapperGetTargets(next->path, &targetPaths) < 0 &&
+            errno != ENOSYS && errno != EBADF) {
+            virReportSystemError(errno,
+                                 _("Unable to get devmapper targets for %s"),
+                                 next->path);
+            return -1;
+        }
+
+        for (i = 0; targetPaths && targetPaths[i]; i++) {
+            if (qemuDomainCreateDevice(targetPaths[i], data, false) < 0)
+                return -1;
+        }
     }
 
     /* qemu-pr-helper might require access to /dev/mapper/control. */
@@ -12342,39 +12359,43 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,
                              virStorageSourcePtr src)
 {
     virStorageSourcePtr next;
-    const char **paths = NULL;
+    VIR_AUTOSTRINGLIST paths = NULL;
     size_t npaths = 0;
-    char *dmPath = NULL;
-    int ret = -1;
-
-    if (!qemuDomainNamespaceEnabled(vm, QEMU_DOMAIN_NS_MOUNT))
-        return 0;
 
     for (next = src; virStorageSourceIsBacking(next); next = next->backingStore) {
+        VIR_AUTOSTRINGLIST targetPaths = NULL;
+
         if (virStorageSourceIsEmpty(next) ||
             !virStorageSourceIsLocalStorage(next)) {
             /* Not creating device. Just continue. */
             continue;
         }
 
-        if (VIR_APPEND_ELEMENT_COPY(paths, npaths, next->path) < 0)
-            goto cleanup;
+        if (virStringListAdd(&paths, next->path) < 0)
+            return -1;
+
+        if (virDevMapperGetTargets(next->path, &targetPaths) < 0 &&
+            errno != ENOSYS && errno != EBADF) {
+            virReportSystemError(errno,
+                                 _("Unable to get devmapper targets for %s"),
+                                 next->path);
+            return -1;
+        }
+
+        if (virStringListMerge(&paths, &targetPaths) < 0)
+            return -1;
     }
 
     /* qemu-pr-helper might require access to /dev/mapper/control. */
     if (src->pr &&
-        (VIR_STRDUP(dmPath, DEVICE_MAPPER_CONTROL_PATH) < 0 ||
-         VIR_APPEND_ELEMENT_COPY(paths, npaths, dmPath) < 0))
-        goto cleanup;
+        virStringListAdd(&paths, DEVICE_MAPPER_CONTROL_PATH) < 0)
+        return -1;
 
-    if (qemuDomainNamespaceMknodPaths(vm, paths, npaths) < 0)
-        goto cleanup;
+    npaths = virStringListLength((const char **) paths);
+    if (qemuDomainNamespaceMknodPaths(vm, (const char **) paths, npaths) < 0)
+        return -1;
 
-    ret = 0;
- cleanup:
-    VIR_FREE(dmPath);
-    VIR_FREE(paths);
-    return ret;
+    return 0;
 }
 
 
diff --git a/src/util/virdevmapper.h b/src/util/virdevmapper.h
index 34d6655e77..42a7828fde 100644
--- a/src/util/virdevmapper.h
+++ b/src/util/virdevmapper.h
@@ -24,6 +24,8 @@
 #ifndef __VIR_DEVMAPPER_H__
 # define __VIR_DEVMAPPER_H__
 
+#include "internal.h"
+
 int
 virDevMapperGetTargets(const char *path,
                        char ***devPaths);
-- 
2.27.0