[libvirt] [PATCH v6] util: Set SIGPIPE to a no-op handler in virFork
Libvirtd has set SIGPIPE to ignored, and virFork resets all signal handlers to the defaults. But child process may write logs to stderr/stdout, that may generate SIGPIPE if journald has stopped. So set SIGPIPE to a dummy no-op handler before unmask signals in virFork(), and the handler will get reset to SIG_DFL when execve() runs. Now we can delete sigaction() call entirely in virExec(). Signed-off-by: Wang Yechao --- v3 patch: https://www.redhat.com/archives/libvir-list/2019-October/msg00934.html Changes in v4: - don't block SIGPIPE, ignore it when invoke VIR_FORCE_CLOSE and virCommandMassClose Changes in v5: - chang from SIG_IGN to a no-op handler in child process Changes in v6: - add a comment and delete sigaction() call entirely in virExec --- src/util/vircommand.c | 32 ++-- 1 file changed, 10 insertions(+), 22 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 93b3dd2..8b10253 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -217,6 +217,8 @@ virCommandFDSet(virCommandPtr cmd, #ifndef WIN32 +static void virDummyHandler(int sig G_GNUC_UNUSED) {} + /** * virFork: * @@ -312,6 +314,14 @@ virFork(void) ignore_value(sigaction(i, _action, NULL)); } +/* Code that runs between fork & execve might trigger + * SIG_PIPE, so we must explicitly set that to a no-op + * handler. This handler will get reset to SIG_DFL when + * execve() runs + */ +sig_action.sa_handler = virDummyHandler; +ignore_value(sigaction(SIGPIPE, _action, NULL)); + /* Unmask all signals in child, since we've no idea what the * caller's done with their signal mask and don't want to * propagate that to children */ @@ -550,7 +560,6 @@ virExec(virCommandPtr cmd) g_autofree char *binarystr = NULL; const char *binary = NULL; int ret; -struct sigaction waxon, waxoff; g_autofree gid_t *groups = NULL; int ngroups; @@ -718,21 +727,6 @@ virExec(virCommandPtr cmd) } } -/* virFork reset all signal handlers to the defaults. - * This is good for the child process, but our hook - * risks running something that generates SIGPIPE, - * so we need to temporarily block that again - */ -memset(, 0, sizeof(waxoff)); -waxoff.sa_handler = SIG_IGN; -sigemptyset(_mask); -memset(, 0, sizeof(waxon)); -if (sigaction(SIGPIPE, , ) < 0) { -virReportSystemError(errno, "%s", - _("Could not disable SIGPIPE")); -goto fork_error; -} - if (virProcessSetMaxMemLock(0, cmd->maxMemLock) < 0) goto fork_error; if (virProcessSetMaxProcesses(0, cmd->maxProcesses) < 0) @@ -783,12 +777,6 @@ virExec(virCommandPtr cmd) if (virCommandHandshakeChild(cmd) < 0) goto fork_error; -if (sigaction(SIGPIPE, , NULL) < 0) { -virReportSystemError(errno, "%s", - _("Could not re-enable SIGPIPE")); -goto fork_error; -} - /* Close logging again to ensure no FDs leak to child */ virLogReset(); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v5] util: Set SIGPIPE to a no-op handler in virFork
Libvirtd has set SIGPIPE to ignored, and virFork resets all signal handlers to the defaults. But child process may write logs to stderr/stdout, that may generate SIGPIPE if journald has stopped. So set SIGPIPE to a dummy no-op handler before unmask signals in virFork. And there is no need to set SIGPIPE ignored before running hooks in virExec. At last, set SIGPIPE to defaults before execve. Signed-off-by: Wang Yechao --- v3 patch: https://www.redhat.com/archives/libvir-list/2019-October/msg00934.html Changes in v4: - don't block SIGPIPE, ignore it when invoke VIR_FORCE_CLOSE and virCommandMassClose Changes in v5: - chang from SIG_IGN to a no-op handler in child process --- src/util/vircommand.c | 27 ++- 1 file changed, 10 insertions(+), 17 deletions(-) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 93b3dd2..c13739c 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -217,6 +217,8 @@ virCommandFDSet(virCommandPtr cmd, #ifndef WIN32 +static void virDummyHandler(int sig G_GNUC_UNUSED) {} + /** * virFork: * @@ -312,6 +314,9 @@ virFork(void) ignore_value(sigaction(i, _action, NULL)); } +sig_action.sa_handler = virDummyHandler; +ignore_value(sigaction(SIGPIPE, _action, NULL)); + /* Unmask all signals in child, since we've no idea what the * caller's done with their signal mask and don't want to * propagate that to children */ @@ -550,7 +555,7 @@ virExec(virCommandPtr cmd) g_autofree char *binarystr = NULL; const char *binary = NULL; int ret; -struct sigaction waxon, waxoff; +struct sigaction sig_action; g_autofree gid_t *groups = NULL; int ngroups; @@ -718,21 +723,6 @@ virExec(virCommandPtr cmd) } } -/* virFork reset all signal handlers to the defaults. - * This is good for the child process, but our hook - * risks running something that generates SIGPIPE, - * so we need to temporarily block that again - */ -memset(, 0, sizeof(waxoff)); -waxoff.sa_handler = SIG_IGN; -sigemptyset(_mask); -memset(, 0, sizeof(waxon)); -if (sigaction(SIGPIPE, , ) < 0) { -virReportSystemError(errno, "%s", - _("Could not disable SIGPIPE")); -goto fork_error; -} - if (virProcessSetMaxMemLock(0, cmd->maxMemLock) < 0) goto fork_error; if (virProcessSetMaxProcesses(0, cmd->maxProcesses) < 0) @@ -783,7 +773,10 @@ virExec(virCommandPtr cmd) if (virCommandHandshakeChild(cmd) < 0) goto fork_error; -if (sigaction(SIGPIPE, , NULL) < 0) { +memset(_action, 0, sizeof(sig_action)); +sig_action.sa_handler = SIG_DFL; +sigemptyset(_action.sa_mask); +if (sigaction(SIGPIPE, _action, NULL) < 0) { virReportSystemError(errno, "%s", _("Could not re-enable SIGPIPE")); goto fork_error; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4] util: Ignore SIGPIPE before write logs to stderr in child process
Libvirtd has set SIGPIPE to ignored, and virFork resets all signal handlers to the defaults. But child process may write logs to stderr/stdout, that may generate SIGPIPE if journald has stopped. So in virExec(), set SIGPIPE to ignored before invoke virCommandMassClose, and revert it back to SIG_DFL after that. In virProcessRunInFork(), just set SIGPIPE to ignored. Signed-off-by: Wang Yechao --- v1 patch: https://www.redhat.com/archives/libvir-list/2019-October/msg00720.html Changes in v2: - use pthread_sigmask to block SIGPIPE Changes in v3: - pass SIG_UNBLOCK(not SIG_SETMASK) to pthread_sigmask when unlock SIGPIPE Changes in v4: - don't block SIGPIPE, ignore it when invoke VIR_FORCE_CLOSE and virCommandMassClose --- src/util/vircommand.c | 16 src/util/virprocess.c | 10 ++ 2 files changed, 26 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 763bcb6..bb6e73b 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -653,9 +653,25 @@ virExec(virCommandPtr cmd) umask(cmd->mask); ret = EXIT_CANCELED; +memset(, 0, sizeof(waxoff)); +waxoff.sa_handler = SIG_IGN; +sigemptyset(_mask); +memset(, 0, sizeof(waxon)); +if (sigaction(SIGPIPE, , ) < 0) { +virReportSystemError(errno, "%s", + _("Could not disable SIGPIPE")); +goto fork_error; +} + if (virCommandMassClose(cmd, childin, childout, childerr) < 0) goto fork_error; +if (sigaction(SIGPIPE, , NULL) < 0) { +virReportSystemError(errno, "%s", + _("Could not re-enable SIGPIPE")); +goto fork_error; +} + if (prepareStdFd(childin, STDIN_FILENO) < 0) { virReportSystemError(errno, "%s", _("failed to setup stdin file handle")); diff --git a/src/util/virprocess.c b/src/util/virprocess.c index e8d4ba0..da66dcf 100644 --- a/src/util/virprocess.c +++ b/src/util/virprocess.c @@ -1151,6 +1151,16 @@ virProcessRunInFork(virProcessForkCallback cb, goto cleanup; if (child == 0) { +struct sigaction waxoff; + +memset(, 0, sizeof(waxoff)); +waxoff.sa_handler = SIG_IGN; +sigemptyset(_mask); +if (sigaction(SIGPIPE, , NULL) < 0) { +virReportSystemError(errno, "%s", + _("Could not disable SIGPIPE")); +} + VIR_FORCE_CLOSE(errfd[0]); ret = virProcessRunInForkHelper(errfd[1], parent, cb, opaque); VIR_FORCE_CLOSE(errfd[1]); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] util: Block SIGPIPE until execve in child process
Libvirtd has set SIGPIPE to ignored, and virFork resets all signal handlers to the defaults. But child process may write logs to stderr/stdout, that may generate SIGPIPE if journald has stopped. So block SIGPIPE in virFork, and unblock it before execve. Signed-off-by: Wang Yechao --- v1 patch: https://www.redhat.com/archives/libvir-list/2019-October/msg00720.html Changes in v2: - use pthread_sigmask to block SIGPIPE Changes in v3: - pass SIG_UNBLOCK(not SIG_SETMASK) to pthread_sigmask when unlock SIGPIPE --- src/util/vircommand.c | 17 + 1 file changed, 17 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 79e1e87..bd3a582 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -321,6 +321,15 @@ virFork(void) virDispatchError(NULL); _exit(EXIT_CANCELED); } + +sigemptyset(); +sigaddset(, SIGPIPE); +if (pthread_sigmask(SIG_BLOCK, , NULL) != 0) { +virReportSystemError(errno, "%s", _("cannot block SIGPIPE")); +virDispatchError(NULL); +_exit(EXIT_CANCELED); +} + } return pid; } @@ -553,6 +562,7 @@ virExec(virCommandPtr cmd) struct sigaction waxon, waxoff; VIR_AUTOFREE(gid_t *) groups = NULL; int ngroups; +sigset_t set; if (cmd->args[0][0] != '/') { if (!(binary = binarystr = virFindFileInPath(cmd->args[0]))) { @@ -792,6 +802,13 @@ virExec(virCommandPtr cmd) /* Close logging again to ensure no FDs leak to child */ virLogReset(); +sigemptyset(); +sigaddset(, SIGPIPE); +if (pthread_sigmask(SIG_UNBLOCK, , NULL) != 0) { +virReportSystemError(errno, "%s", _("cannot unblock SIGPIPE")); +goto fork_error; +} + if (cmd->env) execve(binary, cmd->args, cmd->env); else -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] util: Block SIGPIPE until execve in child process
Libvirtd has set SIGPIPE to ignored, and virFork resets all signal handlers to the defaults. But child process may write logs to stderr/stdout, that may generate SIGPIPE if journald has stopped. So block SIGPIPE in virFork, and unblock it before execve. Signed-off-by: Wang Yechao --- v1 patch: https://www.redhat.com/archives/libvir-list/2019-October/msg00720.html Changes in v2: - use pthread_sigmask to block SIGPIPE --- src/util/vircommand.c | 17 + 1 file changed, 17 insertions(+) diff --git a/src/util/vircommand.c b/src/util/vircommand.c index 79e1e87..bd3a582 100644 --- a/src/util/vircommand.c +++ b/src/util/vircommand.c @@ -321,6 +321,15 @@ virFork(void) virDispatchError(NULL); _exit(EXIT_CANCELED); } + +sigemptyset(); +sigaddset(, SIGPIPE); +if (pthread_sigmask(SIG_BLOCK, , NULL) != 0) { +virReportSystemError(errno, "%s", _("cannot block SIGPIPE")); +virDispatchError(NULL); +_exit(EXIT_CANCELED); +} + } return pid; } @@ -553,6 +562,7 @@ virExec(virCommandPtr cmd) struct sigaction waxon, waxoff; VIR_AUTOFREE(gid_t *) groups = NULL; int ngroups; +sigset_t set; if (cmd->args[0][0] != '/') { if (!(binary = binarystr = virFindFileInPath(cmd->args[0]))) { @@ -792,6 +802,13 @@ virExec(virCommandPtr cmd) /* Close logging again to ensure no FDs leak to child */ virLogReset(); +sigemptyset(); +sigaddset(, SIGPIPE); +if (pthread_sigmask(SIG_SETMASK, , NULL) != 0) { +virReportSystemError(errno, "%s", _("cannot unblock SIGPIPE")); +goto fork_error; +} + if (cmd->env) execve(binary, cmd->args, cmd->env); else -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: ignore SIGPIPE when writing to stderr/stdout
libvirtd's stderr/stdout redirected to journald by default, so if the journald has stopped, libvirtd and libvirtd's child process will receive SIGPIPE signal when writing logs to stderr/stdout. journald stopped reasons: 1. manual command "systemctl stop systemd-journald.service" 2. oom killer kill it. ... Signed-off-by: Wang Yechao --- src/util/virlog.c | 8 1 file changed, 8 insertions(+) diff --git a/src/util/virlog.c b/src/util/virlog.c index 4c76fbc..127e121 100644 --- a/src/util/virlog.c +++ b/src/util/virlog.c @@ -39,6 +39,7 @@ # include #endif #include +#include #include "virerror.h" #include "virlog.h" @@ -732,6 +733,9 @@ virLogOutputToFd(virLogSourcePtr source ATTRIBUTE_UNUSED, if (fd < 0) return; +if (fd == STDERR_FILENO || fd == STDOUT_FILENO) +signal(SIGPIPE, SIG_IGN); + if (virAsprintfQuiet(, "%s: %s", timestamp, str) < 0) return; @@ -740,6 +744,10 @@ virLogOutputToFd(virLogSourcePtr source ATTRIBUTE_UNUSED, if (flags & VIR_LOG_STACK_TRACE) virLogStackTraceToFd(fd); + +if (fd == STDERR_FILENO || fd == STDOUT_FILENO) +signal(SIGPIPE, SIG_DFL); + } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] util: change the return value of virCgroupRemove if failed
virCgroupRemove return -1 when removing cgroup failed. But there are retry code to remove cgroup in QemuProcessStop: retry: if ((ret = qemuRemoveCgroup(vm)) < 0) { if (ret == -EBUSY && (retries++ < 5)) { usleep(200*1000); goto retry; } VIR_WARN("Failed to remove cgroup for %s", vm->def->name); } The return value of qemuRemoveCgroup will never be equal to "-EBUSY", so change the return value of virCgroupRemove if failed. Signed-off-by: Wang Yechao --- src/util/vircgroup.c | 8 +--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 268e401..260ed2d 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -2399,11 +2399,13 @@ int virCgroupRemove(virCgroupPtr group) { size_t i; +int ret = 0; for (i = 0; i < VIR_CGROUP_BACKEND_TYPE_LAST; i++) { -if (group->backends[i] && -group->backends[i]->remove(group) < 0) { -return -1; +if (group->backends[i]) +ret = group->backends[i]->remove(group); +if (ret < 0) +return ret; } } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Update a domain's persistent definition file if it's defined
From: Li XueLei During making disk snapshot in an active domain, sometimes we should update the domain's persistent definition.We must check if the domain is defined, before we update the persistent definition file. First,we create a vm.Then,define the vm and undefine the vm.Last create a --memspec snapshot for the vm.Now,we will find a persistent definition file in the config directory if we don't use this patch. Signed-off-by: Li XueLei --- src/qemu/qemu_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index e32257f..94dcb97 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -15330,7 +15330,7 @@ qemuDomainSnapshotCreateDiskActive(virQEMUDriverPtr driver, if (ret == 0 || !do_transaction) { if (virDomainSaveStatus(driver->xmlopt, cfg->stateDir, vm, driver->caps) < 0 || -(persist && virDomainSaveConfig(cfg->configDir, driver->caps, +(vm->persistent && persist && virDomainSaveConfig(cfg->configDir, driver->caps, vm->newDef) < 0)) ret = -1; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: Assign device address before qemuDomainSupportsNicdev
qemuDomainSupportsNicdev will check the device address type on aarch64. If it is invoked before device address assigned, hotadd vhostuser interface with no address specified will get error. Let qemuDomainEnsurePCIAddress run before qemuDomainSupportsNicdev. Signed-off-by: Wang Yechao --- v1 patch: https://www.redhat.com/archives/libvir-list/2018-December/msg00435.html Changes in v2: - do not modify the address type, let qemuDomainEnsurePCIAddress do it. --- src/qemu/qemu_hotplug.c | 38 +++--- 1 file changed, 19 insertions(+), 19 deletions(-) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 8da0233..f25c8db 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1369,6 +1369,25 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, if (qemuAssignDeviceNetAlias(vm->def, net, -1) < 0) goto cleanup; +if (qemuDomainIsS390CCW(vm->def) && +net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && +virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) { +net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; +if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def))) +goto cleanup; +if (virDomainCCWAddressAssign(>info, ccwaddrs, + !net->info.addr.ccw.assigned) < 0) +goto cleanup; +} else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { +virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("virtio-s390 net device cannot be hotplugged.")); +goto cleanup; +} else if (qemuDomainEnsurePCIAddress(vm, , driver) < 0) { +goto cleanup; +} + +releaseaddr = true; + switch (actualType) { case VIR_DOMAIN_NET_TYPE_BRIDGE: case VIR_DOMAIN_NET_TYPE_NETWORK: @@ -1503,25 +1522,6 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, goto cleanup; } -if (qemuDomainIsS390CCW(vm->def) && -net->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI && -virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_CCW)) { -net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_CCW; -if (!(ccwaddrs = virDomainCCWAddressSetCreateFromDomain(vm->def))) -goto cleanup; -if (virDomainCCWAddressAssign(>info, ccwaddrs, - !net->info.addr.ccw.assigned) < 0) -goto cleanup; -} else if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_VIRTIO_S390)) { -virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", - _("virtio-s390 net device cannot be hotplugged.")); -goto cleanup; -} else if (qemuDomainEnsurePCIAddress(vm, , driver) < 0) { -goto cleanup; -} - -releaseaddr = true; - if (VIR_ALLOC_N(tapfdName, tapfdSize) < 0 || VIR_ALLOC_N(vhostfdName, vhostfdSize) < 0) goto cleanup; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: Add default address type for vhost-user interface on aarch64
on aarch64, hotadd vhost-user interface with the follow xml file: will get error like that: error: internal error: Nicdev support unavailable Because there is no device address type specified in xml file, so qemuDomainSupportsNicdev returns 'false' when invoked in qemuDomainAttachNetDevice. Using pci as the default address type, and assigns pci address later in qemuDomainEnsurePCIAddress. Signed-off-by: Wang Yechao --- src/qemu/qemu_hotplug.c | 5 + 1 file changed, 5 insertions(+) diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index 68d021a..c1464a9 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1443,6 +1443,11 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, queueSize = net->driver.virtio.queues; if (!queueSize) queueSize = 1; + +if (!net->info.type && +vm->def->os.arch == VIR_ARCH_AARCH64) +net->info.type = VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI; + if (!qemuDomainSupportsNicdev(vm->def, net)) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("Nicdev support unavailable")); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu:address:Fix segfault in qemuDomainPrimeVirtioDeviceAddresses
On aarch64, lauch vm with the follow configuration: libvirtd will crash when access the net->model. Signed-off-by: Wang Yechao --- src/qemu/qemu_domain_address.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain_address.c b/src/qemu/qemu_domain_address.c index 24dd7c1..27c9bfb 100644 --- a/src/qemu/qemu_domain_address.c +++ b/src/qemu/qemu_domain_address.c @@ -324,7 +324,8 @@ qemuDomainPrimeVirtioDeviceAddresses(virDomainDefPtr def, for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; -if (STREQ(net->model, "virtio") && +if (net->model && +STREQ(net->model, "virtio") && net->info.type == VIR_DOMAIN_DEVICE_ADDRESS_TYPE_NONE) { net->info.type = type; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: agent: Avoid agentError when closing the QEMU agent
The commit 89563efc0209b854d2b2e554423423d7602acdbd fix the monitor error when closing the QEMU monitor. The QEMU agent has a problem similar to QEMU monitor. So fix the QEMU agent with the same method. Signed-off-by: Wang Yechao --- Changes in v3: - change the commit messages --- src/qemu/qemu_agent.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 97ad0e7..d842b0e 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -530,6 +530,9 @@ static void qemuAgentUpdateWatch(qemuAgentPtr mon) VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR; +if (!mon->watch) +return; + if (mon->lastError.code == VIR_ERR_OK) { events |= VIR_EVENT_HANDLE_READABLE; @@ -555,6 +558,12 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif +if (mon->fd == -1 || mon->watch == 0) { +virObjectUnlock(mon); +virObjectUnref(mon); +return; +} + if (mon->fd != fd || mon->watch != watch) { if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) eof = true; @@ -788,8 +797,10 @@ void qemuAgentClose(qemuAgentPtr mon) virObjectLock(mon); if (mon->fd >= 0) { -if (mon->watch) +if (mon->watch) { virEventRemoveHandle(mon->watch); +mon->watch = 0; +} VIR_FORCE_CLOSE(mon->fd); } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: agent: Avoid agentError when closing the QEMU agent
After calling qemuAgentClose(), it is still possible for the QEMU Agent I/O event callback to get invoked. This will trigger an agent error because mon->fd has been set to -1 at this point. Then vm->privateData->agentError is always 'true' except that restart libvirtd or restart qemu-guest-agent process in guest. Silently ignore the case where mon->fd is -1, likewise for mon->watch being zero. Signed-off-by: Wang Yechao --- v1 patch: https://www.redhat.com/archives/libvir-list/2018-September/msg01382.html Changes in v2: - do not set agentError, let agent state as disconnected instead of error. --- src/qemu/qemu_agent.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_agent.c b/src/qemu/qemu_agent.c index 97ad0e7..d842b0e 100644 --- a/src/qemu/qemu_agent.c +++ b/src/qemu/qemu_agent.c @@ -530,6 +530,9 @@ static void qemuAgentUpdateWatch(qemuAgentPtr mon) VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR; +if (!mon->watch) +return; + if (mon->lastError.code == VIR_ERR_OK) { events |= VIR_EVENT_HANDLE_READABLE; @@ -555,6 +558,12 @@ qemuAgentIO(int watch, int fd, int events, void *opaque) VIR_DEBUG("Agent %p I/O on watch %d fd %d events %d", mon, watch, fd, events); #endif +if (mon->fd == -1 || mon->watch == 0) { +virObjectUnlock(mon); +virObjectUnref(mon); +return; +} + if (mon->fd != fd || mon->watch != watch) { if (events & (VIR_EVENT_HANDLE_HANGUP | VIR_EVENT_HANDLE_ERROR)) eof = true; @@ -788,8 +797,10 @@ void qemuAgentClose(qemuAgentPtr mon) virObjectLock(mon); if (mon->fd >= 0) { -if (mon->watch) +if (mon->watch) { virEventRemoveHandle(mon->watch); +mon->watch = 0; +} VIR_FORCE_CLOSE(mon->fd); } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: agent: Reset agentError when qemuConnectAgent success
qemuAgentClose and qemuAgentIO have race condition, as follows: main thread: second thread: virEventPollRunOnceprocessSerialChangedEvent virEventPollDispatchHandles virMutexUnlock() qemuAgentClose virObjectLock(mon) virEventRemoveHandle VIR_FORCE_CLOSE(mon->fd) virObjectUnlock(mon) priv->agentError = false qemuAgentIO virObjectLock(mon) mon->fd != fd --> error = true qemuProcessHandleAgentError priv->agentError = true virObjectUnlock(mon) virMutexLock() qemuAgentClose set the mon->fd to '-1', and then qemuAgentIO check the mon->fd not equals to fd that registered before. qemuProcessHandleAgentError will be called to set priv->agentError to 'true', then the priv->agentError is always 'true' except restart libvirtd or restart qemu-guest-agent process in guest. We can't send any qemu-agent-command anymore even if qemuConnectAgent return success later. This is accidently occurs when hot-add-vcpu in windows2012. virsh setvcpus ... virsh qemu-agent-command $vm '{"execute":"guest-get-vcpus"}' Reset the priv->agentError to 'false' when qemuConnectAgent sucess to fix this problem. Signed-off-by: Wang Yechao --- src/qemu/qemu_process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 29b0ba1..4fbb955 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -269,6 +269,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) virResetLastError(); } +priv->agentError = false; return 0; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 0/4] qemu: Fix deadlock if create qemuProcessReconnect thread failed
v3 patch: https://www.redhat.com/archives/libvir-list/2018-September/msg00950.html --- Changes in v4: - Split up v3 into four parts Wang Yechao (4): qemu: Split up qemuDomainRemoveInactive qemu: Create a qemuDomainRemoveInactiveLocked qemu: Create a qemuDomainRemoveInactiveJobLocked qemu: Fix deadlock if create qemuProcessReconnect thread failed src/qemu/qemu_domain.c | 79 - src/qemu/qemu_domain.h | 3 ++ src/qemu/qemu_process.c | 2 +- 3 files changed, 69 insertions(+), 15 deletions(-) -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 4/4] qemu: Fix deadlock if create qemuProcessReconnect thread failed
qemuProcessReconnectHelper has hold lock on doms, if create qemuProcessReconnect thread failed, calls qemuDomainRemoveInactiveJob will lead to deadlock. Modify qemuProcessReconnectHelper to call qemuDomainRemoveInactiveJobLocked. Signed-off-by: Wang Yechao --- src/qemu/qemu_process.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f9a01da..711db10 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8033,7 +8033,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj, */ qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, QEMU_ASYNC_JOB_NONE, 0); -qemuDomainRemoveInactiveJob(src->driver, obj); +qemuDomainRemoveInactiveJobLocked(src->driver, obj); virDomainObjEndAPI(); virNWFilterUnlockFilterUpdates(); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 3/4] qemu: Create a qemuDomainRemoveInactiveJobLocked
Create a qemuDomainRemoveInactiveJobLocked which copies qemuDomainRemoveInactiveJob except of course calling virDomainObjListRemoveLocked. Signed-off-by: Wang Yechao --- src/qemu/qemu_domain.c | 21 + src/qemu/qemu_domain.h | 3 +++ 2 files changed, 24 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 22436d2..b20d430 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8438,6 +8438,27 @@ qemuDomainRemoveInactiveJob(virQEMUDriverPtr driver, } +/** + * qemuDomainRemoveInactiveJobLocked: + * + * Similar to qemuDomainRemoveInactiveJob, except that the caller must + * also hold the lock @driver->domains + */ +void +qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ +bool haveJob; + +haveJob = qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) >= 0; + +qemuDomainRemoveInactiveLocked(driver, vm); + +if (haveJob) +qemuDomainObjEndJob(driver, vm); +} + + void qemuDomainSetFakeReboot(virQEMUDriverPtr driver, virDomainObjPtr vm, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 914c9a6..ecbe2e8 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -704,6 +704,9 @@ void qemuDomainRemoveInactive(virQEMUDriverPtr driver, void qemuDomainRemoveInactiveJob(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver, + virDomainObjPtr vm); + void qemuDomainSetFakeReboot(virQEMUDriverPtr driver, virDomainObjPtr vm, bool value); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 1/4] qemu: Split up qemuDomainRemoveInactive
Split up qemuDomainRemoveInactive into qemuDomainRemoveInactiveCommon and virDomainObjListRemove Signed-off-by: Wang Yechao --- src/qemu/qemu_domain.c | 38 +++--- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2fd8a2a..191113a 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8344,23 +8344,13 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, } -/** - * qemuDomainRemoveInactive: - * - * The caller must hold a lock to the vm. - */ -void -qemuDomainRemoveInactive(virQEMUDriverPtr driver, - virDomainObjPtr vm) +static void +qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, + virDomainObjPtr vm) { char *snapDir; virQEMUDriverConfigPtr cfg; -if (vm->persistent) { -/* Short-circuit, we don't want to remove a persistent domain */ -return; -} - cfg = virQEMUDriverGetConfig(driver); /* Remove any snapshot metadata prior to removing the domain */ @@ -8379,13 +8369,31 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, } qemuExtDevicesCleanupHost(driver, vm->def); -virDomainObjListRemove(driver->domains, vm); - virObjectUnref(cfg); } /** + * qemuDomainRemoveInactive: + * + * The caller must hold a lock to the vm. + */ +void +qemuDomainRemoveInactive(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ +if (vm->persistent) { +/* Short-circuit, we don't want to remove a persistent domain */ +return; +} + +qemuDomainRemoveInactiveCommon(driver, vm); + +virDomainObjListRemove(driver->domains, vm); +} + + +/** * qemuDomainRemoveInactiveJob: * * Just like qemuDomainRemoveInactive but it tries to grab a -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v4 2/4] qemu: Create a qemuDomainRemoveInactiveLocked
Create a qemuDomainRemoveInactiveLocked which is a copy of qemuDomainRemoveInactive except that instead of calling virDomainObjListRemove it calls virDomainObjListRemoveLocked. Signed-off-by: Wang Yechao --- src/qemu/qemu_domain.c | 22 ++ 1 file changed, 22 insertions(+) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 191113a..22436d2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8394,6 +8394,28 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, /** + * qemuDomainRemoveInactiveLocked: + * + * The caller must hold a lock to the vm and must hold the + * lock on driver->domains in order to call the remove obj + * from locked list method. + */ +static void +qemuDomainRemoveInactiveLocked(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ +if (vm->persistent) { +/* Short-circuit, we don't want to remove a persistent domain */ +return; +} + +qemuDomainRemoveInactiveCommon(driver, vm); + +virDomainObjListRemoveLocked(driver->domains, vm); +} + + +/** * qemuDomainRemoveInactiveJob: * * Just like qemuDomainRemoveInactive but it tries to grab a -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v3] qemu: fix deadlock if create qemuProcessReconnect thread failed
qemuProcessReconnectHelper has hold lock on doms, if create qemuProcessReconnect thread failed, calls qemuDomainRemoveInactiveJob will lead to deadlock. Add a qemuDomainRemoveInactiveJobLocked, modify qemuProcessReconnectHelper to call it. Signed-off-by: Wang Yechao --- v2 patch: https://www.redhat.com/archives/libvir-list/2018-September/msg00635.html Changes in v3: - drop v2 patch, cause it is not good. - split qemuDomainRemoveInactive into qemuDomainRemoveInactiveCommon and virDomainObjListRemove. - create a qemuDomainRemoveInactiveLocked, consists of qemuDomainRemoveInactiveCommon and virDomainObjListRemoveLocked. - create a qemuDomainRemoveInactiveJobLocked, which copies qemuDomainRemoveInactiveJob except calling qemuDomainRemoveInactiveLocked - Modify qemuProcessReconnectHelper to call qemuDomainRemoveInactiveJobLocked --- src/qemu/qemu_domain.c | 75 - src/qemu/qemu_domain.h | 9 ++ src/qemu/qemu_process.c | 2 +- 3 files changed, 71 insertions(+), 15 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 2fd8a2a..ef0d91d 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -8343,24 +8343,13 @@ qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, return rem.err; } - -/** - * qemuDomainRemoveInactive: - * - * The caller must hold a lock to the vm. - */ void -qemuDomainRemoveInactive(virQEMUDriverPtr driver, - virDomainObjPtr vm) +qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, + virDomainObjPtr vm) { char *snapDir; virQEMUDriverConfigPtr cfg; -if (vm->persistent) { -/* Short-circuit, we don't want to remove a persistent domain */ -return; -} - cfg = virQEMUDriverGetConfig(driver); /* Remove any snapshot metadata prior to removing the domain */ @@ -8379,9 +8368,47 @@ qemuDomainRemoveInactive(virQEMUDriverPtr driver, } qemuExtDevicesCleanupHost(driver, vm->def); +virObjectUnref(cfg); +} + +/** + * qemuDomainRemoveInactive: + * + * The caller must hold a lock to the vm. + */ +void +qemuDomainRemoveInactive(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ +if (vm->persistent) { +/* Short-circuit, we don't want to remove a persistent domain */ +return; +} + +qemuDomainRemoveInactiveCommon(driver, vm); virDomainObjListRemove(driver->domains, vm); -virObjectUnref(cfg); +} + +/** + * qemuDomainRemoveInactiveLocked: + * + * The caller must hold a lock to the vm , + * and hold a lock to the doms. + */ + +void +qemuDomainRemoveInactiveLocked(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ +if (vm->persistent) { +/* Short-circuit, we don't want to remove a persistent domain */ +return; +} + +qemuDomainRemoveInactiveCommon(driver, vm); +virDomainObjListRemoveLocked(driver->domains, vm); + } @@ -8407,6 +8434,26 @@ qemuDomainRemoveInactiveJob(virQEMUDriverPtr driver, qemuDomainObjEndJob(driver, vm); } +/** + * qemuDomainRemoveInactiveJobLocked: + * + * Just like qemuDomainRemoveInactiveJob but it hold lock + * on 'doms'. + */ + +void +qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver, + virDomainObjPtr vm) +{ +bool haveJob; + +haveJob = qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) >= 0; + +qemuDomainRemoveInactiveLocked(driver, vm); + +if (haveJob) +qemuDomainObjEndJob(driver, vm); +} void qemuDomainSetFakeReboot(virQEMUDriverPtr driver, diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h index 914c9a6..1d80621 100644 --- a/src/qemu/qemu_domain.h +++ b/src/qemu/qemu_domain.h @@ -698,12 +698,21 @@ int qemuDomainSnapshotDiscardAll(void *payload, int qemuDomainSnapshotDiscardAllMetadata(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainRemoveInactiveCommon(virQEMUDriverPtr driver, +virDomainObjPtr vm); + void qemuDomainRemoveInactive(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainRemoveInactiveLocked(virQEMUDriverPtr driver, +virDomainObjPtr vm); + void qemuDomainRemoveInactiveJob(virQEMUDriverPtr driver, virDomainObjPtr vm); +void qemuDomainRemoveInactiveJobLocked(virQEMUDriverPtr driver, + virDomainObjPtr vm); + void qemuDomainSetFakeReboot(virQEMUDriverPtr driver, virDomainObjPtr vm, bool value); diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 72a59de..ed24447 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8025,7 +8025,7 @@ qemuProcessReconnectHelpe
[libvirt] [PATCH v2] numa: fix unsafe access to numa_nodes_ptr
numa_nodes_ptr is a global variable in libnuma.so. It is been freed after main thread exits. If we have many running vms, restart the libvirtd service continuously at intervals of a few seconds, the main thread may exit before qemuProcessReconnect thread, and a segfault error occurs. Backstrace as follows: 0 0x7f40e3d2dd72 in numa_bitmask_isbitset () from /lib64/libnuma.so.1 1 0x7f40e4d14c55 in virNumaNodeIsAvailable (node=node@entry=0) at util/virnuma.c:396 2 0x7f40e4d16010 in virNumaGetHostMemoryNodeset () at util/virnuma.c:1011 3 0x7f40b94ced90 in qemuRestoreCgroupState (vm=0x7f407c39df00, vm=0x7f407c39df00) at qemu/qemu_cgroup.c:877 4 qemuConnectCgroup (driver=driver@entry=0x7f407c21fe80, vm=0x7f407c39df00) at qemu/qemu_cgroup.c:969 5 0x7f40b94eef93 in qemuProcessReconnect (opaque=) at qemu/qemu_process.c:3531 6 0x7f40e4d34bd2 in virThreadHelper (data=) at util/virthread.c:206 7 0x7f40e214ee25 in start_thread () from /lib64/libpthread.so.0 8 0x7f40e1e7c36d in clone () from /lib64/libc.so.6 Signed-off-by: Wang Yechao --- src/util/virnuma.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 67e6c86..f06f6b3 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -381,7 +381,10 @@ virNumaGetMaxCPUs(void) bool virNumaNodeIsAvailable(int node) { -return numa_bitmask_isbitset(numa_nodes_ptr, node); +if (numa_nodes_ptr) +return numa_bitmask_isbitset(numa_nodes_ptr, node); +else +return false; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] qemu: fix deadlock if create qemuProcessReconnect thread failed
qemuProcessReconnectHelper has hold the doms lock, if create qemuProcessReconnect thread failed, it will get the doms lock again to remove the dom from doms list. add obj->inReconnetCtx flag to avoid deadlock. Signed-off-by: Wang Yechao --- src/conf/domain_conf.h | 1 + src/conf/virdomainobjlist.c | 6 -- src/qemu/qemu_process.c | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e30a4b2..5bc5771 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2608,6 +2608,7 @@ struct _virDomainObj { virDomainSnapshotObjPtr current_snapshot; bool hasManagedSave; +bool inReconnectCtx; void *privateData; void (*privateDataFreeFunc)(void *); diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 805fe94..30300b4 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -397,11 +397,13 @@ virDomainObjListRemove(virDomainObjListPtr doms, dom->removing = true; virObjectRef(dom); virObjectUnlock(dom); -virObjectRWLockWrite(doms); +if (!dom->inReconnectCtx) +virObjectRWLockWrite(doms); virObjectLock(dom); virDomainObjListRemoveLocked(doms, dom); virObjectUnref(dom); -virObjectRWUnlock(doms); +if (!dom->inReconnectCtx) +virObjectRWUnlock(doms); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index eb9904b..8c30850 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8029,6 +8029,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj, */ qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, QEMU_ASYNC_JOB_NONE, 0); +obj->inReconnectCtx = true; qemuDomainRemoveInactiveJob(src->driver, obj); virDomainObjEndAPI(); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH v2] nwfilter: fix deadlock when nwfilter reload
user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite() step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao Reviewed-by: Wang Yi --- src/nwfilter/nwfilter_driver.c | 10 +- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 1ee5162..ab85072 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -80,18 +80,26 @@ static void nwfilterDriverUnlock(void) } #if HAVE_FIREWALLD +static void nwfilterReloadThread(void *opaque ATTRIBUTE_UNUSED) +{ +nwfilterStateReload(); +} static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { +virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); -nwfilterStateReload(); + +if (virThreadCreate(, false, nwfilterReloadThread, NULL) < 0) +VIR_WARN("create nwfilterReloadThread failed."); } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] numa: fix unsafe access to numa_nodes_ptr
numa_nodes_ptr is a global variable in libnuma.so. It is been freed after main thread exits. If we have many running vms, restart the libvirtd service continuously at intervals of a few seconds, the main thread may exit before qemuProcessReconnect thread, and a segfault error occurs. Backstrace as follows: 0 0x7f40e3d2dd72 in numa_bitmask_isbitset () from /lib64/libnuma.so.1 1 0x7f40e4d14c55 in virNumaNodeIsAvailable (node=node@entry=0) at util/virnuma.c:396 2 0x7f40e4d16010 in virNumaGetHostMemoryNodeset () at util/virnuma.c:1011 3 0x7f40b94ced90 in qemuRestoreCgroupState (vm=0x7f407c39df00, vm=0x7f407c39df00) at qemu/qemu_cgroup.c:877 4 qemuConnectCgroup (driver=driver@entry=0x7f407c21fe80, vm=0x7f407c39df00) at qemu/qemu_cgroup.c:969 5 0x7f40b94eef93 in qemuProcessReconnect (opaque=) at qemu/qemu_process.c:3531 6 0x7f40e4d34bd2 in virThreadHelper (data=) at util/virthread.c:206 7 0x7f40e214ee25 in start_thread () from /lib64/libpthread.so.0 8 0x7f40e1e7c36d in clone () from /lib64/libc.so.6 Signed-off-by: Wang Yechao --- src/util/virnuma.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/util/virnuma.c b/src/util/virnuma.c index 67e6c86..502b1d5 100644 --- a/src/util/virnuma.c +++ b/src/util/virnuma.c @@ -381,7 +381,10 @@ virNumaGetMaxCPUs(void) bool virNumaNodeIsAvailable(int node) { -return numa_bitmask_isbitset(numa_nodes_ptr, node); +if (numa_nodes_ptr) +return numa_bitmask_isbitset(numa_nodes_ptr, node); +else +return false; } -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu: fix deadlock if create qemuProcessReconnect thread failed
qemuProcessReconnectHelper has hold the doms lock, if create qemuProcessReconnect thread failed, it will get the doms lock again to remove the dom from doms list. add obj->inReconnetCtx flag to avoid deadlock. Signed-off-by: Wang Yechao --- src/conf/domain_conf.h | 1 + src/conf/virdomainobjlist.c | 6 -- src/qemu/qemu_process.c | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index e30a4b2..5bc5771 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -2608,6 +2608,7 @@ struct _virDomainObj { virDomainSnapshotObjPtr current_snapshot; bool hasManagedSave; +bool inReconnectCtx; void *privateData; void (*privateDataFreeFunc)(void *); diff --git a/src/conf/virdomainobjlist.c b/src/conf/virdomainobjlist.c index 805fe94..30300b4 100644 --- a/src/conf/virdomainobjlist.c +++ b/src/conf/virdomainobjlist.c @@ -397,11 +397,13 @@ virDomainObjListRemove(virDomainObjListPtr doms, dom->removing = true; virObjectRef(dom); virObjectUnlock(dom); -virObjectRWLockWrite(doms); +if (!dom->inReconnectCtx) +virObjectRWLockWrite(doms); virObjectLock(dom); virDomainObjListRemoveLocked(doms, dom); virObjectUnref(dom); -virObjectRWUnlock(doms); +if (!dom->inReconnectCtx) +virObjectRWUnlock(doms); } diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index eb9904b..8c30850 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -8029,6 +8029,7 @@ qemuProcessReconnectHelper(virDomainObjPtr obj, */ qemuProcessStop(src->driver, obj, VIR_DOMAIN_SHUTOFF_FAILED, QEMU_ASYNC_JOB_NONE, 0); +obj->inReconnectCtx = true; qemuDomainRemoveInactiveJob(src->driver, obj); virDomainObjEndAPI(); -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter:fix deadlock when nwfilter reload
user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite() step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao Reviewed-by: Yi Wang --- src/nwfilter/nwfilter_driver.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) mode change 100644 => 100755 src/nwfilter/nwfilter_driver.c diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c old mode 100644 new mode 100755 index ac3a964..2c099e2 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,17 +81,28 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void nwfilterThread(void *opaque ATTRIBUTE_UNUSED) +{ +nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { +virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); -nwfilterStateReload(); + +/* create thread handle the nwfilter reload, don't block the event_loop.*/ +if (virThreadCreate(, false, nwfilterThread, NULL) < 0) { +VIR_ERROR("create nwfilterThread failed."); +} } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter: fix deadlock when nwfilter reload
user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite() step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao Reviewed-by: Wang Yi --- src/nwfilter/nwfilter_driver.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c index 1ee5162..8dcc40b 100644 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -80,18 +80,27 @@ static void nwfilterDriverUnlock(void) } #if HAVE_FIREWALLD +static void nwfilterReloadThread(void *opaque ATTRIBUTE_UNUSED) +{ +nwfilterStateReload(); +} static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { +virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); -nwfilterStateReload(); + +if (virThreadCreate(, false, nwfilterReloadThread, NULL) < 0) { +VIR_ERROR("create nwfilterThread failed."); +} } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter:fix deadlock when nwfilter reload
user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite() step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao Reviewed-by: Wang Yi --- src/nwfilter/nwfilter_driver.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) mode change 100644 => 100755 src/nwfilter/nwfilter_driver.c diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c old mode 100644 new mode 100755 index ac3a964..2c099e2 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,17 +81,28 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void nwfilterThread(void *opaque ATTRIBUTE_UNUSED) +{ +nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { +virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); -nwfilterStateReload(); + +/* create thread handle the nwfilter reload, don't block the event_loop.*/ +if (virThreadCreate(, false, nwfilterThread, NULL) < 0) { +VIR_ERROR("create nwfilterThread failed."); +} } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter:fix deadlock when nwfilter reload
user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite() step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao Reviewed-by: Wang Yi --- src/nwfilter/nwfilter_driver.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) mode change 100644 => 100755 src/nwfilter/nwfilter_driver.c diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c old mode 100644 new mode 100755 index ac3a964..2c099e2 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,17 +81,28 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void nwfilterThread(void *opaque ATTRIBUTE_UNUSED) +{ +nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { +virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); -nwfilterStateReload(); + +/* create thread handle the nwfilter reload, don't block the event_loop.*/ +if (virThreadCreate(, false, nwfilterThread, NULL) < 0) { +VIR_ERROR("create nwfilterThread failed."); +} } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] nwfilter:fix deadlock when nwfilter reload
user run "firewalld-cmd --reload" nwfilterStateReload called in main thread step 1. virRWLockWrite() step 2. virNWFilterLoadAllConfigs step 3. virRWLockUnlock(); lauch a vm: qemuDomainCreateXML runs in other thread step 1. virRWLockRead(); step 2. qemuProcessStart step 3. qemuProcessWaitForMonitor step 4. ... step 5 virRWLockUnlock(); if nwfilterStateReload called in the middle of step 1 and step 5 of qemuDomainCreateXML, it can't get the updateLock and then block the event_loop, so event_loop can't handle the qemu-monitor messages, cause deadlock move nwfilterStateReload into thread to fix this problem. Signed-off-by: Wang Yechao Reviewed-by: Wang Yi --- src/nwfilter/nwfilter_driver.c | 13 - 1 file changed, 12 insertions(+), 1 deletion(-) mode change 100644 => 100755 src/nwfilter/nwfilter_driver.c diff --git a/src/nwfilter/nwfilter_driver.c b/src/nwfilter/nwfilter_driver.c old mode 100644 new mode 100755 index ac3a964..2c099e2 --- a/src/nwfilter/nwfilter_driver.c +++ b/src/nwfilter/nwfilter_driver.c @@ -81,17 +81,28 @@ static void nwfilterDriverUnlock(void) #if HAVE_FIREWALLD +static void nwfilterThread(void *opaque ATTRIBUTE_UNUSED) +{ +nwfilterStateReload(); +} + static DBusHandlerResult nwfilterFirewalldDBusFilter(DBusConnection *connection ATTRIBUTE_UNUSED, DBusMessage *message, void *user_data ATTRIBUTE_UNUSED) { +virThread thread; + if (dbus_message_is_signal(message, DBUS_INTERFACE_DBUS, "NameOwnerChanged") || dbus_message_is_signal(message, "org.fedoraproject.FirewallD1", "Reloaded")) { VIR_DEBUG("Reload in nwfilter_driver because of firewalld."); -nwfilterStateReload(); + +/* create thread handle the nwfilter reload, don't block the event_loop.*/ +if (virThreadCreate(, false, nwfilterThread, NULL) < 0) { +VIR_ERROR("create nwfilterThread failed."); +} } return DBUS_HANDLER_RESULT_NOT_YET_HANDLED; -- 1.8.3.1 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list