[PATCH] qemu: pre-create the dbus directory in qemuStateInitialize
From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001 From: Bihong Yu <yubihong@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@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
On a Monday in 2020, Bihong Yu wrote:
From 187323ce736dcd9b1a177d552630b0c6859a4798 Mon Sep 17 00:00:00 2001 From: Bihong Yu <yubihong@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@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
From 165abdd77c7db83ebf98232b80d6b988471185c0 Mon Sep 17 00:00:00 2001 From: Bihong Yu <yubihong@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@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
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
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.
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@redhat.com> Jano
participants (3)
-
Bihong Yu -
Ján Tomko -
Pino Toscano