>From 099707463944faeae75da640b0d1d780cb675406 Mon Sep 17 00:00:00 2001
From: Bihong Yu <yubihong@huawei.com>
Date: Tue, 16 Jun 2020 22:08:55 +0800
Subject: [PATCH] utils: check flags and errno before reporting errno

There are races condiction to make '/run/libvirt/qemu/dbus' directory in
virDirCreateNoFork() while concurrent start VMs, and get "failed to create
directory '/run/libvirt/qemu/dbus': File exists" error message. Check flags and
errno before reporting errno after mkdir().

Signed-off-by:Bihong Yu <yubihong@huawei.com>
Reviewed-by:Chuan Zheng <zhengchuan@huawei.com>
Reviewed-by:alexchen <alex.chen@huawei.com>
---
 src/util/virfile.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/util/virfile.c b/src/util/virfile.c
index 20260a2..96ac4e5 100644
--- a/src/util/virfile.c
+++ b/src/util/virfile.c
@@ -2614,12 +2614,15 @@ virDirCreateNoFork(const char *path,

     if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) {
         if (mkdir(path, mode) < 0) {
-            ret = -errno;
-            virReportSystemError(errno, _("failed to create directory '%s'"),
-                                 path);
-            goto error;
+            if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && errno == EEXIST)) {
+                ret = -errno;
+                virReportSystemError(errno, _("failed to create directory '%s'"),
+                                     path);
+                goto error;
+            }
+        } else {
+            created = true;
         }
-        created = true;
     }

     if (stat(path, &st) == -1) {
-- 
1.8.3.1

On a Wednesday in 2020, Bihong Yu wrote:
>>From 099707463944faeae75da640b0d1d780cb675406 Mon Sep 17 00:00:00 2001
>From: Bihong Yu <yubihong@huawei.com>
>Date: Tue, 16 Jun 2020 22:08:55 +0800
>Subject: [PATCH] utils: check flags and errno before reporting errno
>
>There are races condiction to make '/run/libvirt/qemu/dbus' directory in
>virDirCreateNoFork() while concurrent start VMs, and get "failed to create
>directory '/run/libvirt/qemu/dbus': File exists" error message. Check flags and
>errno before reporting errno after mkdir().
>

Sigh. That's because we use yet another per-daemon directory instead of
the existing per-VM one:
https://www.redhat.com/archives/libvir-list/2020-February/msg01134.html

IMO the easy fix here is to pre-create the directory in qemuStateInitialize
like we do for all the other directories.

The right fix would be to rename the per-vm "libdir" to "rundir" and
move it to /run, since that reflects its true usage more.

(That would require some backwards-compatibility code, see
qemuDomainSetPrivatePathsOld)

>Signed-off-by:Bihong Yu <yubihong@huawei.com>
>Reviewed-by:Chuan Zheng <zhengchuan@huawei.com>
>Reviewed-by:alexchen <alex.chen@huawei.com>
>---
> src/util/virfile.c | 13 ++++++++-----
> 1 file changed, 8 insertions(+), 5 deletions(-)
>
>diff --git a/src/util/virfile.c b/src/util/virfile.c
>index 20260a2..96ac4e5 100644
>--- a/src/util/virfile.c
>+++ b/src/util/virfile.c
>@@ -2614,12 +2614,15 @@ virDirCreateNoFork(const char *path,
>
>     if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) {
>         if (mkdir(path, mode) < 0) {
>-            ret = -errno;
>-            virReportSystemError(errno, _("failed to create directory '%s'"),
>-                                 path);
>-            goto error;
>+            if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && errno == EEXIST)) {
>+                ret = -errno;
>+                virReportSystemError(errno, _("failed to create directory '%s'"),
>+                                     path);
>+                goto error;

With this change, we don't need to call virFileExists in the condition
above. But I'd still keep the one in the huge condition in virDirCreate,
as it can avoid a fork:

     if ((!(flags & VIR_DIR_CREATE_AS_UID))
         || (geteuid() != 0)
         || ((uid == (uid_t) -1) && (gid == (gid_t) -1))
         || ((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) {
         return virDirCreateNoFork(path, mode, uid, gid, flags);
     }

Jano

>+            }
>+        } else {
>+            created = true;
>         }
>-        created = true;
>     }
>
>     if (stat(path, &st) == -1) {
>-- 
>1.8.3.1
>