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

[Ján Tomko]

On a Wednesday in 2020, Michal Privoznik wrote:
>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
>

That bug is private.

>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);

This changes the functions that are moved in the very next patch.
Doing changes after the movement makes 'git blame' easier to use.

Jano
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20200724/7c3f1939/attachment-0001.sig>