src/util/virfile.c | 7 +++++++ 1 file changed, 7 insertions(+)
>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
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 >
© 2016 - 2024 Red Hat, Inc.