On a Wednesday in 2020, Bihong Yu wrote:
From 099707463944faeae75da640b0d1d780cb675406 Mon Sep 17 00:00:00 2001From: Bihong Yu <yubihong@xxxxxxxxxx> 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@xxxxxxxxxx>
Reviewed-by:Chuan Zheng <zhengchuan@xxxxxxxxxx>
Reviewed-by:alexchen <alex.chen@xxxxxxxxxx>
---
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
Attachment:
signature.asc
Description: PGP signature
