[libvirt PATCH] utils: add mutex to avoid races in virfile
From d9f7ed2af581222804392f9b93dc6aaf7d8c8995 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: add mutex to avoid races in virfile
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. Add an mutex to avoid races. 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 | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/util/virfile.c b/src/util/virfile.c index 20260a2..ae02a6e 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -88,6 +88,7 @@ #include "virstring.h" #include "virutil.h" #include "virsocket.h" +#include "virthread.h" #define VIR_FROM_THIS VIR_FROM_NONE @@ -108,6 +109,9 @@ VIR_LOG_INIT("util.file"); # define O_DIRECT 0 #endif +/* Global mutex to avoid races */ +virMutex fileLock = VIR_MUTEX_INITIALIZER; + int virFileClose(int *fdptr, virFileCloseFlags flags) { int saved_errno = 0; @@ -2612,15 +2616,18 @@ virDirCreateNoFork(const char *path, struct stat st; bool created = false; + virMutexLock(&fileLock); if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) { if (mkdir(path, mode) < 0) { ret = -errno; virReportSystemError(errno, _("failed to create directory '%s'"), path); + virMutexUnlock(&fileLock); goto error; } created = true; } + virMutexUnlock(&fileLock); if (stat(path, &st) == -1) { ret = -errno; -- 1.8.3.1
On Tue, Jun 16, 2020 at 10:39:46PM +0800, Bihong Yu wrote:
From d9f7ed2af581222804392f9b93dc6aaf7d8c8995 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: add mutex to avoid races in virfile
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. Add an mutex to avoid races.
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 | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/util/virfile.c b/src/util/virfile.c index 20260a2..ae02a6e 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -88,6 +88,7 @@ #include "virstring.h" #include "virutil.h" #include "virsocket.h" +#include "virthread.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -108,6 +109,9 @@ VIR_LOG_INIT("util.file"); # define O_DIRECT 0 #endif
+/* Global mutex to avoid races */ +virMutex fileLock = VIR_MUTEX_INITIALIZER; + int virFileClose(int *fdptr, virFileCloseFlags flags) { int saved_errno = 0; @@ -2612,15 +2616,18 @@ virDirCreateNoFork(const char *path, struct stat st; bool created = false;
+ virMutexLock(&fileLock); if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) { if (mkdir(path, mode) < 0) {
Instead of adding a mutex we should get rid of the virFileExists check entirely. Instead check "errno == EEXIST && (flags & VIR_DIR_CREATE_ALLOW_EXIST)" before reporting an error.
ret = -errno; virReportSystemError(errno, _("failed to create directory '%s'"), path); + virMutexUnlock(&fileLock); goto error; } created = true; } + virMutexUnlock(&fileLock);
if (stat(path, &st) == -1) { ret = -errno;
Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
Yes, it a good idea. I will fix it. On 2020/6/16 23:16, Daniel P. Berrangé wrote:
On Tue, Jun 16, 2020 at 10:39:46PM +0800, Bihong Yu wrote:
From d9f7ed2af581222804392f9b93dc6aaf7d8c8995 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: add mutex to avoid races in virfile
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. Add an mutex to avoid races.
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 | 7 +++++++ 1 file changed, 7 insertions(+)
diff --git a/src/util/virfile.c b/src/util/virfile.c index 20260a2..ae02a6e 100644 --- a/src/util/virfile.c +++ b/src/util/virfile.c @@ -88,6 +88,7 @@ #include "virstring.h" #include "virutil.h" #include "virsocket.h" +#include "virthread.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -108,6 +109,9 @@ VIR_LOG_INIT("util.file"); # define O_DIRECT 0 #endif
+/* Global mutex to avoid races */ +virMutex fileLock = VIR_MUTEX_INITIALIZER; + int virFileClose(int *fdptr, virFileCloseFlags flags) { int saved_errno = 0; @@ -2612,15 +2616,18 @@ virDirCreateNoFork(const char *path, struct stat st; bool created = false;
+ virMutexLock(&fileLock); if (!((flags & VIR_DIR_CREATE_ALLOW_EXIST) && virFileExists(path))) { if (mkdir(path, mode) < 0) {
Instead of adding a mutex we should get rid of the virFileExists check entirely. Instead check "errno == EEXIST && (flags & VIR_DIR_CREATE_ALLOW_EXIST)" before reporting an error.
ret = -errno; virReportSystemError(errno, _("failed to create directory '%s'"), path); + virMutexUnlock(&fileLock); goto error; } created = true; } + virMutexUnlock(&fileLock);
if (stat(path, &st) == -1) { ret = -errno;
Regards, Daniel
participants (2)
-
Bihong Yu -
Daniel P. Berrangé