[PATCH] utils: check flags and errno before reporting errno

Ján Tomko jtomko at redhat.com
Wed Jun 17 12:07:08 UTC 2020


On a Wednesday in 2020, Bihong Yu wrote:
>>From 099707463944faeae75da640b0d1d780cb675406 Mon Sep 17 00:00:00 2001
>From: Bihong Yu <yubihong at 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 at huawei.com>
>Reviewed-by:Chuan Zheng <zhengchuan at huawei.com>
>Reviewed-by:alexchen <alex.chen at 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
>
-------------- 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/20200617/ff862281/attachment-0001.sig>


More information about the libvir-list mailing list