1. Patchew
  2. Libvirt
  3. View series

[libvirt PATCH] utils: add mutex to avoid races in virfile

Bihong Yu posted 1 patch 5 years, 7 months ago
Failed in applying to current master (apply log)
src/util/virfile.c | 7 +++++++
1 file changed, 7 insertions(+)
[libvirt PATCH] utils: add mutex to avoid races in virfile
Posted by Bihong Yu 5 years, 7 months ago 
>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
Re: [libvirt PATCH] utils: add mutex to avoid races in virfile
Posted by Daniel P. Berrangé 5 years, 7 months ago 
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 :|
Re: [libvirt PATCH] utils: add mutex to avoid races in virfile
Posted by Bihong Yu 5 years, 7 months ago 
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
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.