[PATCH] utils: check flags and errno before reporting errno
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
participants (2)
-
Bihong Yu -
Ján Tomko