[PATCH v1 02/34] virDevMapperGetTargets: Don't ignore EBADF

Michal Privoznik mprivozn at redhat.com
Wed Jul 22 09:39:56 UTC 2020


One of the symptoms of the bug [1] is that on the second start of
a domain we get EBADF when talking to libdevmapper. The reason is
that libdevmapper opens /dev/mapper/control to talk to kernel and
saves the FD into a global variable. This works well when
starting a domain for the first time: the pre-exec hook (which is
a separate process) gets info it needs; then the daemon sets up
CGroups (where it will open the file again, because it's a
different process). Now, libdevmapper won't close this FD until
library is unloaded (in destructor) or dm_lib_release() is
called. We were not doing any of that, hence, when starting a
domain (any domain, even a different one), we forked off a child
process (which will eventually become QEMU), mass close all FDs
(including the libdevmapper's one), and run pre-exec hook. Since
we closed the FD, libdevmapper will pass closed FD into an
ioctl() and thus we got EBADF.

After previous patch, this approach is history and thus we
should not see EBADF anymore.

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

Signed-off-by: Michal Privoznik <mprivozn at redhat.com>
---
 src/qemu/qemu_cgroup.c  | 2 +-
 src/qemu/qemu_domain.c  | 4 ++--
 src/util/virdevmapper.c | 3 ---
 3 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 914bf640ca..e88da02341 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -87,7 +87,7 @@ qemuSetupImagePathCgroup(virDomainObjPtr vm,
     }
 
     if (virDevMapperGetTargets(path, &targetPaths) < 0 &&
-        errno != ENOSYS && errno != EBADF) {
+        errno != ENOSYS) {
         virReportSystemError(errno,
                              _("Unable to get devmapper targets for %s"),
                              path);
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 5b22eb2eaa..2058290870 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -10264,7 +10264,7 @@ qemuDomainSetupDisk(virQEMUDriverConfigPtr cfg G_GNUC_UNUSED,
                 return -1;
 
             if (virDevMapperGetTargets(next->path, &targetPaths) < 0 &&
-                errno != ENOSYS && errno != EBADF) {
+                errno != ENOSYS) {
                 virReportSystemError(errno,
                                      _("Unable to get devmapper targets for %s"),
                                      next->path);
@@ -11328,7 +11328,7 @@ qemuDomainNamespaceSetupDisk(virDomainObjPtr vm,
             tmpPath = g_strdup(next->path);
 
             if (virDevMapperGetTargets(next->path, &targetPaths) < 0 &&
-                errno != ENOSYS && errno != EBADF) {
+                errno != ENOSYS) {
                 virReportSystemError(errno,
                                      _("Unable to get devmapper targets for %s"),
                                      next->path);
diff --git a/src/util/virdevmapper.c b/src/util/virdevmapper.c
index 1c216fb6c1..139d70b854 100644
--- a/src/util/virdevmapper.c
+++ b/src/util/virdevmapper.c
@@ -176,9 +176,6 @@ virDevMapperGetTargetsImpl(const char *path,
  * If @path consists of yet another devmapper targets these are
  * consulted recursively.
  *
- * If we don't have permissions to talk to kernel, -1 is returned
- * and errno is set to EBADF.
- *
  * Returns 0 on success,
  *        -1 otherwise (with errno set, no libvirt error is
  *        reported)
-- 
2.26.2




More information about the libvir-list mailing list