3:15 a.m.
From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001
From: Bihong Yu <yubihong(a)huawei.com>
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 <yubihong(a)huawei.com>
---
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
8:49 a.m.
On a Monday in 2020, Bihong Yu wrote:
>From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00
2001
From: Bihong Yu <yubihong(a)huawei.com>
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 <yubihong(a)huawei.com>
---
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
9:09 p.m.
New subject: [PATCH V2] qemu: pre-create the dbus directory in qemuStateInitialize
From 165abdd77c7db83ebf98232b80d6b988471185c0 Mon Sep 17 00:00:00 2001
From: Bihong Yu <yubihong(a)huawei.com>
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 <yubihong(a)huawei.com>
---
src/qemu/qemu_dbus.c | 10 ----------
src/qemu/qemu_dbus.h | 2 --
src/qemu/qemu_driver.c | 7 +++++++
src/qemu/qemu_process.c | 3 ---
4 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/src/qemu/qemu_dbus.c b/src/qemu/qemu_dbus.c
index 51f6c94..8104287 100644
--- a/src/qemu/qemu_dbus.c
+++ b/src/qemu/qemu_dbus.c
@@ -33,16 +33,6 @@
VIR_LOG_INIT("qemu.dbus");
-int
-qemuDBusPrepareHost(virQEMUDriverPtr driver)
-{
- g_autoptr(virQEMUDriverConfig) cfg = virQEMUDriverGetConfig(driver);
-
- return virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
- VIR_DIR_CREATE_ALLOW_EXIST);
-}
-
-
static char *
qemuDBusCreatePidFilename(virQEMUDriverConfigPtr cfg,
const char *shortName)
diff --git a/src/qemu/qemu_dbus.h b/src/qemu/qemu_dbus.h
index 474eb10..3c2145a 100644
--- a/src/qemu/qemu_dbus.h
+++ b/src/qemu/qemu_dbus.h
@@ -21,8 +21,6 @@
#include "qemu_conf.h"
#include "qemu_domain.h"
-int qemuDBusPrepareHost(virQEMUDriverPtr driver);
-
char *qemuDBusGetAddress(virQEMUDriverPtr driver,
virDomainObjPtr vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 8e81c30..53980d4 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -743,6 +743,13 @@ qemuStateInitialize(bool privileged,
goto error;
}
+ if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
+ virReportSystemError(errno, _("Failed to create dbus state dir %s"),
+ cfg->dbusStateDir);
+ goto error;
+ }
+
if ((qemu_driver->lockFD =
virPidFileAcquire(cfg->stateDir, "driver", false, getpid())) <
0)
goto error;
diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index e8b15ee..1006f41 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -6466,9 +6466,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
12:36 a.m.
New subject: [PATCH V2] qemu: pre-create the dbus directory in qemuStateInitialize
On Wednesday, 22 July 2020 04:09:24 CEST Bihong Yu wrote:
+ if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user,
cfg->group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
+ virReportSystemError(errno, _("Failed to create dbus state dir %s"),
+ cfg->dbusStateDir);
Minor notes on the message:
- spell "D-Bus" correctly
- no need to abbreviate "directory"
- quote the path placeholder
so I suggest something like:
"Failed to create the D-Bus state directory '%s'"
(Can't comment on the rest of the changes, sorry.)
--
Pino Toscano
1:54 a.m.
New subject: [PATCH V2] qemu: pre-create the dbus directory in qemuStateInitialize
On 2020/7/22 13:36, Pino Toscano wrote:
On Wednesday, 22 July 2020 04:09:24 CEST Bihong Yu wrote:
> + if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
> + VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
> + virReportSystemError(errno, _("Failed to create dbus state dir
%s"),
> + cfg->dbusStateDir);
Minor notes on the message:
- spell "D-Bus" correctly
- no need to abbreviate "directory"
- quote the path placeholder
so I suggest something like:
"Failed to create the D-Bus state directory '%s'"
Sorry, the error message is written with reference to other contexts of
qemuStateInitialize(), such as:
if (virFileMakePath(cfg->memoryBackingDir) < 0) {
virReportSystemError(errno, _("Failed to create memory backing dir
%s"),
cfg->memoryBackingDir);
goto error;
}
if (virFileMakePath(cfg->slirpStateDir) < 0) {
virReportSystemError(errno, _("Failed to create slirp state dir %s"),
cfg->slirpStateDir);
goto error;
}
+ if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
+ virReportSystemError(errno, _("Failed to create dbus state dir %s"),
+ cfg->dbusStateDir);
+ goto error;
+ }
So I don't think that's a good suggestion. If you still insist your suggestion, I
will rewrite it.
2:54 a.m.
New subject: [PATCH V2] qemu: pre-create the dbus directory in qemuStateInitialize
On a Wednesday in 2020, Bihong Yu wrote:
On 2020/7/22 13:36, Pino Toscano wrote:
> On Wednesday, 22 July 2020 04:09:24 CEST Bihong Yu wrote:
>> + if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
>> + VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
>> + virReportSystemError(errno, _("Failed to create dbus state dir
%s"),
>> + cfg->dbusStateDir);
>
> Minor notes on the message:
> - spell "D-Bus" correctly
> - no need to abbreviate "directory"
> - quote the path placeholder
> so I suggest something like:
> "Failed to create the D-Bus state directory '%s'"
Sorry, the error message is written with reference to other contexts of
qemuStateInitialize(), such as:
if (virFileMakePath(cfg->memoryBackingDir) < 0) {
virReportSystemError(errno, _("Failed to create memory backing dir
%s"),
cfg->memoryBackingDir);
goto error;
}
if (virFileMakePath(cfg->slirpStateDir) < 0) {
virReportSystemError(errno, _("Failed to create slirp state dir %s"),
cfg->slirpStateDir);
goto error;
}
+ if (virDirCreate(cfg->dbusStateDir, 0770, cfg->user, cfg->group,
+ VIR_DIR_CREATE_ALLOW_EXIST) < 0) {
+ virReportSystemError(errno, _("Failed to create dbus state dir %s"),
+ cfg->dbusStateDir);
+ goto error;
+ }
So I don't think that's a good suggestion. If you still insist your suggestion, I
will rewrite it.
They are good suggestions but I'm afraid they're out of scope of
this patch.
Reviewed-by: Ján Tomko <jtomko(a)redhat.com>
Jano
1621
days inactive
1623
days old
5 comments
3 participants
participants (3)
-
Bihong Yu -
Ján Tomko -
Pino Toscano