Re: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

2020-07-21 Thread Ján Tomko 

On a Monday in 2020, Bihong Yu wrote:

From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001

From: Bihong Yu 
Date: Tue, 14 Jul 2020 15:44:05 +0800
Subject: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

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. pre-create the
dbus directory in qemuStateInitialize.

Signed-off-by:Bihong Yu 
---
src/qemu/qemu_dbus.c| 4 +---
src/qemu/qemu_dbus.h| 2 +-
src/qemu/qemu_driver.c  | 4 
src/qemu/qemu_process.c | 3 ---
4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 51f6c94..0e0306a 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -34,10 +34,8 @@ VIR_LOG_INIT("qemu.dbus");


int
-qemuDBusPrepareHost(virQEMUDriverPtr driver)
+qemuDBusPreparePath(virQEMUDriverConfigPtr cfg)


Instead of renaming this function, we can just remove it completely


{
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-



return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
VIR_DIR_CREATE_ALLOW_EXIST);


This virDirCreate call would then fit nicely after 
virFileMakePath(cfg->slirpStateDir),
which is where all the other directories are created.


}


Jano


signature.asc
Description: PGP signature

[PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

2020-07-20 Thread Bihong Yu 
>From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001
From: Bihong Yu 
Date: Tue, 14 Jul 2020 15:44:05 +0800
Subject: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

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. pre-create the
dbus directory in qemuStateInitialize.

Signed-off-by:Bihong Yu 
---
 src/qemu/qemu_dbus.c| 4 +---
 src/qemu/qemu_dbus.h| 2 +-
 src/qemu/qemu_driver.c  | 4 
 src/qemu/qemu_process.c | 3 ---
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 51f6c94..0e0306a 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -34,10 +34,8 @@ VIR_LOG_INIT("qemu.dbus");


 int
-qemuDBusPrepareHost(virQEMUDriverPtr driver)
+qemuDBusPreparePath(virQEMUDriverConfigPtr cfg)
 {
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-
 return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
 VIR_DIR_CREATE_ALLOW_EXIST);
 }
diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h
index 474eb10..6ce9f7b 100644
--- a/src/qemu/qemu_dbus.h
+++ b/src/qemu/qemu_dbus.h
@@ -21,7 +21,7 @@
 #include "qemu_conf.h"
 #include "qemu_domain.h"

-int qemuDBusPrepareHost(virQEMUDriverPtr driver);
+int qemuDBusPreparePath(virQEMUDriverConfigPtr cfg);

 char *qemuDBusGetAddress(virQEMUDriverPtr driver,
  virDomainObjPtr vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d185666..52b68c9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -50,6 +50,7 @@
 #include "qemu_security.h"
 #include "qemu_checkpoint.h"
 #include "qemu_backup.h"
+#include "qemu_dbus.h"

 #include "virerror.h"
 #include "virlog.h"
@@ -790,6 +791,9 @@ qemuStateInitialize(bool privileged,
   cfg->migrationPortMax)) == NULL)
 goto error;

+if (qemuDBusPreparePath(cfg) < 0)
+goto error;
+
 if (qemuSecurityInit(qemu_driver) < 0)
 goto error;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index eba14ed..46620ca 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6449,9 +6449,6 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);

-if (qemuDBusPrepareHost(driver) < 0)
-return -1;
-
 if (qemuPrepareNVRAM(cfg, vm) < 0)
 return -1;

-- 
1.8.3.1

Re: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

2020-07-16 Thread Bihong Yu 
Can anyone help me review the code?

On 2020/7/14 15:53, Bihong Yu wrote:
>>From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001
> From: Bihong Yu 
> Date: Tue, 14 Jul 2020 15:44:05 +0800
> Subject: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize
> 
> 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. pre-create the
> dbus directory in qemuStateInitialize.
> 
> Signed-off-by:Bihong Yu 
> ---
>  src/qemu/qemu_dbus.c| 4 +---
>  src/qemu/qemu_dbus.h| 2 +-
>  src/qemu/qemu_driver.c  | 4 
>  src/qemu/qemu_process.c | 3 ---
>  4 files changed, 6 insertions(+), 7 deletions(-)
> 
> diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
> index 51f6c94..0e0306a 100644
> --- a/src/qemu/qemu_dbus.c
> +++ b/src/qemu/qemu_dbus.c
> @@ -34,10 +34,8 @@ VIR_LOG_INIT("qemu.dbus");
> 
> 
>  int
> -qemuDBusPrepareHost(virQEMUDriverPtr driver)
> +qemuDBusPreparePath(virQEMUDriverConfigPtr cfg)
>  {
> -g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> -
>  return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
>  VIR_DIR_CREATE_ALLOW_EXIST);
>  }
> diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h
> index 474eb10..6ce9f7b 100644
> --- a/src/qemu/qemu_dbus.h
> +++ b/src/qemu/qemu_dbus.h
> @@ -21,7 +21,7 @@
>  #include "qemu_conf.h"
>  #include "qemu_domain.h"
> 
> -int qemuDBusPrepareHost(virQEMUDriverPtr driver);
> +int qemuDBusPreparePath(virQEMUDriverConfigPtr cfg);
> 
>  char *qemuDBusGetAddress(virQEMUDriverPtr driver,
>   virDomainObjPtr vm);
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index d185666..52b68c9 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -50,6 +50,7 @@
>  #include "qemu_security.h"
>  #include "qemu_checkpoint.h"
>  #include "qemu_backup.h"
> +#include "qemu_dbus.h"
> 
>  #include "virerror.h"
>  #include "virlog.h"
> @@ -790,6 +791,9 @@ qemuStateInitialize(bool privileged,
>cfg->migrationPortMax)) == NULL)
>  goto error;
> 
> +if (qemuDBusPreparePath(cfg) < 0)
> +goto error;
> +
>  if (qemuSecurityInit(qemu_driver) < 0)
>  goto error;
> 
> diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
> index eba14ed..46620ca 100644
> --- a/src/qemu/qemu_process.c
> +++ b/src/qemu/qemu_process.c
> @@ -6449,9 +6449,6 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
>  qemuDomainObjPrivatePtr priv = vm->privateData;
>  g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
> 
> -if (qemuDBusPrepareHost(driver) < 0)
> -return -1;
> -
>  if (qemuPrepareNVRAM(cfg, vm) < 0)
>  return -1;
>

[PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

2020-07-14 Thread Bihong Yu 
>From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001
From: Bihong Yu 
Date: Tue, 14 Jul 2020 15:44:05 +0800
Subject: [PATCH] qemu: pre-create the dbus directory in qemuStateInitialize

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. pre-create the
dbus directory in qemuStateInitialize.

Signed-off-by:Bihong Yu 
---
 src/qemu/qemu_dbus.c| 4 +---
 src/qemu/qemu_dbus.h| 2 +-
 src/qemu/qemu_driver.c  | 4 
 src/qemu/qemu_process.c | 3 ---
 4 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 51f6c94..0e0306a 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -34,10 +34,8 @@ VIR_LOG_INIT("qemu.dbus");


 int
-qemuDBusPrepareHost(virQEMUDriverPtr driver)
+qemuDBusPreparePath(virQEMUDriverConfigPtr cfg)
 {
-g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-
 return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
 VIR_DIR_CREATE_ALLOW_EXIST);
 }
diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h
index 474eb10..6ce9f7b 100644
--- a/src/qemu/qemu_dbus.h
+++ b/src/qemu/qemu_dbus.h
@@ -21,7 +21,7 @@
 #include "qemu_conf.h"
 #include "qemu_domain.h"

-int qemuDBusPrepareHost(virQEMUDriverPtr driver);
+int qemuDBusPreparePath(virQEMUDriverConfigPtr cfg);

 char *qemuDBusGetAddress(virQEMUDriverPtr driver,
  virDomainObjPtr vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d185666..52b68c9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -50,6 +50,7 @@
 #include "qemu_security.h"
 #include "qemu_checkpoint.h"
 #include "qemu_backup.h"
+#include "qemu_dbus.h"

 #include "virerror.h"
 #include "virlog.h"
@@ -790,6 +791,9 @@ qemuStateInitialize(bool privileged,
   cfg->migrationPortMax)) == NULL)
 goto error;

+if (qemuDBusPreparePath(cfg) < 0)
+goto error;
+
 if (qemuSecurityInit(qemu_driver) < 0)
 goto error;

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index eba14ed..46620ca 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6449,9 +6449,6 @@ qemuProcessPrepareHost(virQEMUDriverPtr driver,
 qemuDomainObjPrivatePtr priv = vm->privateData;
 g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);

-if (qemuDBusPrepareHost(driver) < 0)
-return -1;
-
 if (qemuPrepareNVRAM(cfg, vm) < 0)
 return -1;

-- 
1.8.3.1