From 29a17260cc1875640e62f63bfe7fa9661dc9fe70 Mon Sep 17 00:00:00 2001

Message-Id: <29a17260cc1875640e62f63bfe7fa9661dc9fe70@dist-git>

From: Michal Privoznik <mprivozn@redhat.com>

Date: Wed, 20 Nov 2019 14:34:11 +0100

Subject: [PATCH] qemu: Forcibly mknod() even if it exists

MIME-Version: 1.0

Content-Type: text/plain; charset=UTF-8

Content-Transfer-Encoding: 8bit

Another weird bug appeared concerning qemu namespaces. Basically

the problem is as follows:

1) Issue an API that causes libvirt to create a node in domain's

   namespace, say /dev/nvme0n1 with 8:0 as major:minor (the API can

   be attach-disk for instance). Or simply create the node from a

   console by hand.

2) Detach the disk from qemu.

3) Do something that makes /dev/nvme0n1 change it's minor number.

4) Try to attach the disk again.

The problem is, in a few cases - like disk-detach - we don't

remove the corresponding /dev node from the mount namespace

(because it may be used by some other disk's backing chain). But

this creates a problem, because if the node changes its MAJ:MIN

numbers we don't propagate the change into the domain's

namespace. We do plain mknod() and ignore EEXIST which obviously

is not enough because it doesn't guarantee that the node has

updated MAJ:MIN pair.

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

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>

(cherry picked from commit cdd8a6690ee3fa4b4b8ca1d4531924bd33be136a)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

Message-Id: <058a9025fdcb817d3334e6c5a7c6a4fe1e95e761.1574256841.git.mprivozn@redhat.com>

Reviewed-by: Jiri Denemark <jdenemar@redhat.com>

---

 src/qemu/qemu_domain.c | 25 +++++++++----------------

 1 file changed, 9 insertions(+), 16 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c

index e7d7a3baeb..3c67769771 100644

--- a/src/qemu/qemu_domain.c

+++ b/src/qemu/qemu_domain.c

@@ -11191,16 +11191,14 @@ qemuDomainCreateDeviceRecursive(const char *device,

                                             allow_noent, ttl - 1) < 0)

             goto cleanup;

     } else if (isDev) {

-        if (create &&

-            mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) {

-            if (errno == EEXIST) {

-                ret = 0;

-            } else {

+        if (create) {

+            unlink(devicePath);

+            if (mknod(devicePath, sb.st_mode, sb.st_rdev) < 0) {

                 virReportSystemError(errno,

                                      _("Failed to make device %s"),

                                      devicePath);

+                goto cleanup;

             }

-            goto cleanup;

         }

     } else if (isReg) {

         if (create &&

@@ -11969,17 +11967,12 @@ qemuDomainAttachDeviceMknodHelper(pid_t pid ATTRIBUTE_UNUSED,

     } else if (isDev) {

         VIR_DEBUG("Creating dev %s (%d,%d)",

                   data->file, major(data->sb.st_rdev), minor(data->sb.st_rdev));

+        unlink(data->file);

         if (mknod(data->file, data->sb.st_mode, data->sb.st_rdev) < 0) {

-            /* Because we are not removing devices on hotunplug, or

-             * we might be creating part of backing chain that

-             * already exist due to a different disk plugged to

-             * domain, accept EEXIST. */

-            if (errno != EEXIST) {

-                virReportSystemError(errno,

-                                     _("Unable to create device %s"),

-                                     data->file);

-                goto cleanup;

-            }

+            virReportSystemError(errno,

+                                 _("Unable to create device %s"),

+                                 data->file);

+            goto cleanup;

         } else {

             delDevice = true;

         }

-- 

2.24.0